fix(extensions): address #264 review follow-ups

[stack72]

Summary

• Add binaries to the belt-and-suspenders path traversal check — every other manifest field had both Zod and fallback validation, binaries was missed
• Add cross-field dedup between additionalFiles and binaries — reject manifests listing the same path in both since both land in files/
• Narrow pull-side chmod catch to only swallow NotFound and log other errors at debug level

Addresses review feedback from #1350.

Test plan

• deno check — passes
• deno lint — passes
• deno fmt — passes
• deno run test — 5731 pass, 0 failures

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR makes no user-visible UX changes beyond adding a single new UserError. The new message (Path "${bf}" appears in both binaries and additionalFiles...) is clear, actionable, and consistent in tone and format with the surrounding error messages in resolve_extension_files.ts. The chmod catch narrowing in pull.ts is debug-level only and not visible to normal users. No commands, flags, help text, or JSON output shapes were changed.

[github-actions]

Adversarial Review

Critical / High

None.

Medium

None.

Low

1. src/cli/resolve_extension_files.ts:534 — binaries not added to seenNormalized: The cross-field dedup checks binaries against seenNormalized (additionalFiles map) but does not insert binaries into seenNormalized. If a future field also lands in files/ and only checks seenNormalized, it would miss collisions with binaries. Not a bug today — only additionalFiles and binaries target files/, and the check between them is complete — but worth noting for defensiveness.

2. src/libswamp/extensions/pull.ts:970-971 — non-NotFound chmod errors logged at debug, not warned: If chmod fails with e.g. PermissionDenied, the binary won't be executable at runtime, but this is only logged at debug level. A user who doesn't run with debug logging would see no indication the binary won't work. The old code was strictly worse (silent catch-all), so this is an improvement, but warn might be more appropriate for non-NotFound chmod failures since they indicate the binary will be broken.

Verdict

PASS — Small, focused fix that plugs real gaps identified in prior review. The path-traversal belt-and-suspenders addition for binaries is correct and follows the established pattern. The cross-field dedup between additionalFiles and binaries is complete (additionalFiles fully populates seenNormalized before binaries checks against it). The chmod error narrowing is strictly better than the previous catch-all. No logic errors, no security issues, no missing edge cases.

[github-actions]

Code Review

All three changes are correct, well-scoped, and improve the security posture of the extension system.

Blocking Issues

None.

Suggestions

1. Test coverage for the new cross-field dedup (resolve_extension_files.ts:526-533): The new validation that rejects manifests listing the same path in both binaries and additionalFiles is genuinely new logic with a distinct error path. The test file has 29 tests but none exercise any binaries validation at all (a pre-existing gap). A test asserting that a manifest with a path in both fields throws the expected UserError would lock this down. Not blocking since the logic is straightforward and correct on inspection, but worth adding.

Notes

• Path traversal fix (line 153): Correctly closes the gap where binaries was the only manifest field missing from the belt-and-suspenders isSafeRelativePath check. Good security fix.
• Cross-field dedup (lines 526-533): The one-directional check against seenNormalized is correct since additionalFiles (step 15) is always processed before binaries (step 16). Error message is clear and actionable.
• Narrowed chmod catch (pull.ts:968-973): Good improvement — only swallowing NotFound instead of all errors, with debug logging for unexpected failures. Follows the LogTape tagged template convention from CLAUDE.md.
• License headers present on both files. No libswamp import boundary violations. No any types introduced.