Context
Follow-up to the link_resolver._resolve_path containment hardening landed in the dogfooding PR (commit 79389ab on feat/792-dogfood-apm, closes #695 / #792). Surfaced during the supply-chain-security review of that change -- not exploitable in the current call paths, but worth fixing so we don't regress later.
Findings
Minor 1 -- empty-string input returns base_path instead of None
_resolve_path("") follows the Path(base_dir) / "" branch and returns the base directory. Not exploitable today because every caller does an .exists() check on the returned path, but semantically wrong (an empty link should resolve to nothing).
Fix: add an early guard at the top of _resolve_path:
if not path or not path.strip():
return None
Minor 2 -- whitespace-only input (same class)
_resolve_path(" ") hits the same code path as empty-string. The same guard covers it.
Minor 3 -- regression test gaps
Add test cases for:
- empty string (
"")
- whitespace-only (
" ")
- embedded NUL byte
- backslash traversal on POSIX (
foo\\..\\..\\etc\\passwd)
file:// URI on POSIX
- non-existent relative target (the happy path of "link resolves but target missing")
Current containment code handles these correctly; tests lock in that behaviour.
Scope
- Single file:
src/apm_cli/compilation/link_resolver.py
- Single test file:
tests/unit/compilation/test_link_resolver.py
- No API change, no behaviour change for valid inputs.
Labels
good first issue, compilation, security-hardening
Context
Follow-up to the
link_resolver._resolve_pathcontainment hardening landed in the dogfooding PR (commit79389abonfeat/792-dogfood-apm, closes #695 / #792). Surfaced during the supply-chain-security review of that change -- not exploitable in the current call paths, but worth fixing so we don't regress later.Findings
Minor 1 -- empty-string input returns
base_pathinstead ofNone_resolve_path("")follows thePath(base_dir) / ""branch and returns the base directory. Not exploitable today because every caller does an.exists()check on the returned path, but semantically wrong (an empty link should resolve to nothing).Fix: add an early guard at the top of
_resolve_path:Minor 2 -- whitespace-only input (same class)
_resolve_path(" ")hits the same code path as empty-string. The same guard covers it.Minor 3 -- regression test gaps
Add test cases for:
"")" ")foo\\..\\..\\etc\\passwd)file://URI on POSIXCurrent containment code handles these correctly; tests lock in that behaviour.
Scope
src/apm_cli/compilation/link_resolver.pytests/unit/compilation/test_link_resolver.pyLabels
good first issue,compilation,security-hardening