Fix path traversal in local file fetch

[campoy]

Summary

• Extract a checkPath function in embedmd/embedmd.go that verifies a local path stays within the base directory using filepath.Abs on both sides of the comparison.
• Call it from embedder.runCommand before delegating to e.Fetch, so the check applies to every embed directive regardless of which Fetcher implementation is in use — including custom fetchers supplied via WithFetcher.
• The check is intentionally not inside fetcher.Fetch in content.go. Placing it there would only protect the default fetcher; any WithFetcher(...) caller would bypass it. Centralising it in runCommand means it runs once for every command, unconditionally.
• mixedContentProvider.Fetch in the test file no longer needs to duplicate the logic either.

A directive like [embedmd]:# (../../../etc/passwd) previously allowed reading arbitrary files accessible to the running user.

Test plan

• path traversal rejected when base dir is set — ../secret.go with a base dir returns an error
• deeply nested path traversal rejected — ../../../secret.go with a nested base dir returns an error
• normal relative path within base dir still works — code.go with a matching base dir succeeds (regression)
• Integration test passes — embedmd docs.md in sample/ correctly resolves hello.go relative to .
• All existing tests continue to pass (go test ./...)

Closes #80

[campoy]

The fix is sound and well-structured. Here is a detailed assessment.

Correctness of checkPath

The core logic is correct. Appending string(os.PathSeparator) to both sides before the HasPrefix comparison is the right technique — it prevents a directory like /tmp/docs from being fooled by a sibling path like /tmp/docs-evil. Using filepath.Abs (rather than just filepath.Clean) correctly handles relative dir values by anchoring to the real working directory.

One gap worth calling out: the check is skipped when dir == "" (line 100–102). This is the default when no WithBaseDir option is passed. In that case, embedmd resolves paths relative to the process working directory, and a traversal directive like [embedmd]:# (../../etc/passwd) will still read arbitrary files. This is arguably the same behaviour as before the fix (no regression), but it is worth documenting explicitly in the function comment. Something like:

// When dir is empty, no base-directory constraint is in force and any relative
// path is resolved against the process working directory; no check is performed.

Symlinks are not covered. filepath.Abs calls filepath.Clean under the hood; it does not resolve symlinks. A symlink inside the base directory that points outside it will pass the check. Fixing this properly requires filepath.EvalSymlinks, but that comes with its own portability and error-handling costs. This is a known limitation of the approach (not unique to this PR), and it is acceptable to leave it for a follow-up — but it should be documented.

Edge cases

Case	Behaviour	Correct?
../secret.go with dir="sample"	Blocked	✓
../../etc/passwd with dir="sample"	Blocked	✓
../../../secret.go with dir="a/b/c"	Blocked	✓
Absolute path /etc/passwd with relative dir	Allowed — filepath.Join("sample", "/etc/passwd") on Unix strips the leading slash, producing sample/etc/passwd	✓ correct
Absolute path /etc/passwd with absolute dir	Allowed — filepath.Join("/tmp/docs", "/etc/passwd") produces /tmp/docs/etc/passwd, safely inside	✓ correct
dir="" + any traversal	Allowed (early return)	⚠ acceptable but undocumented
Symlink inside base dir pointing outside	Allowed	⚠ known limitation
dir is exact string prefix of escaped path (sam vs sample)	Blocked correctly — separator-suffix trick works	✓

Placement of the check in runCommand

The rationale in the PR description is sound. Placing the guard in runCommand rather than in fetcher.Fetch means it is enforced for every Fetcher implementation, including custom ones supplied via WithFetcher. This is the right level of abstraction.

Test quality

Three new table-driven cases are added to TestProcess:

• path traversal rejected when base dir is set — covers the basic reported case.
• deeply nested path traversal rejected — covers deeper nesting.
• normal relative path within base dir still works — regression guard.

These are good, but there is no direct unit test for checkPath itself. Since checkPath is an unexported function containing the entire security logic, a TestCheckPath table-driven test would make it much easier to exhaustively cover edge cases (empty dir, absolute path, the separator-trick, etc.) without going through the full Process pipeline. This is not a blocker but it is a meaningful coverage gap.

One minor observation: the "deeply nested path traversal rejected" test case has no files map. This is intentional and correct — the error fires in checkPath before the fetcher is ever called — but a brief comment to that effect would help future readers.

Duplication / design

The refactor of mixedContentProvider.Fetch is a good cleanup. Renaming path = filepath.Join(...) to resolved := filepath.Join(...) avoids shadowing the parameter, which was a latent readability issue.

The content.go change (collapsing two lines into one) is fine and removes a now-unnecessary intermediate variable.

Error messages

path %q escapes base directory is clear and actionable. The wrapping in runCommand produces could not read ../secret.go: path "../secret.go" escapes base directory — the path appears twice, which is slightly redundant, but it is consistent with how all other errors in runCommand are wrapped and not worth changing in isolation.

Summary

The security fix is correct for the common case and the implementation is clean. Two things are worth addressing before or shortly after merge:

1. Document the dir == "" case in the checkPath comment — either explicitly accepting that traversal is unconstrained without WithBaseDir, or deciding to treat the process cwd as the implicit base and block traversal unconditionally.
2. Add a TestCheckPath unit test to give the security-critical function its own focused coverage.

Neither is a blocker for the fix itself. The change is a clear improvement over the status quo.