Skip to content

fix(tar-extract): judge the entry name that is written; bound pax work per archive - #218

Merged
PrzemekGalarowicz merged 1 commit into
mainfrom
claude/optimistic-heisenberg-8rw8ke
Sep 24, 2026
Merged

PrzemekGalarowicz merged 1 commit into
mainfrom
claude/optimistic-heisenberg-8rw8ke

Conversation

@PrzemekGalarowicz

Copy link
Copy Markdown
Contributor

What this changes

Fix 4 of the post-review batch (review of PHARN-01..18), shipped through /pharn-dev-ship.

  • Name check matches the written name. ustar name/prefix are now decoded as strict UTF-8 (TextDecoder fatal: true, ignoreBOM: true). Before, a lenient decode turned invalid bytes (e.g. raw C1 0x85) into U+FFFD, so hasUnsafeChars judged a different string from the one written. Invalid UTF-8 or a leading BOM now refuses the archive.
  • Pax work bounded per archive. Global (g) pax headers total at most 64 KiB per archive, and each one counts toward maxEntries. Before, the limits applied per header, so the total was unbounded.
  • No raw header bytes in messages. The non-octal numeric-field and bad-UTF-8 errors render bytes through describeBytes, which escapes them as \xNN and caps the length.

Type of change

  • feat — new stack option, wizard step, or command capability
  • fix — bug fix
  • docs — docs-only change
  • chore / refactor — tooling or internal restructure, no behavior change

Area(s) touched

src/lib/tar-extract.ts (fetch boundary), tests/tar-extract.test.ts, CHANGELOG.md (### Security), .dev/features/tar-entry-names/ (pipeline artifacts).

Checklist

  • Read the existing file(s) before editing; followed the ESM .js-extension import convention.
  • Updated the matching tests/*.test.ts — 9 new cases; 8 fail on the base code, and the BOM case is a guard.
  • Updated the relevant docs/ page — N/A (no user-facing surface beyond CHANGELOG).
  • Preserved the security invariants — this change only narrows what an untrusted archive may contain.

Quality gates

  • npm run check passes locally (format:check + lint + typecheck + test) — 1507 tests.
  • npm run build succeeds.
  • npm run test:coverage passes (coverage thresholds met).

Notes for the reviewer

  • Pipeline results: validate exit 0, regress no-regressions, verify PASS, review GREEN with 2 minor advisory findings. See .dev/features/tar-entry-names/REVIEW.md.
  • A real git-archive of pharn-oss still extracts to the same 2208 files / 414 dirs.
  • Advisory: a single non-UTF-8 filename upstream would now fail every install. pharn-oss uses only ASCII names today.

🤖 Generated with Claude Code

https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o


Generated by Claude Code

…k per archive

- Decode ustar name/prefix as strict UTF-8 (ignoreBOM) so the unsafe-character
  check, the path rules and the write all see the same string; a non-UTF-8 or
  BOM-led name refuses the archive instead of reaching the filesystem as U+FFFD.
- Cap total pax global-header bytes per archive at 64 KiB and count global
  headers toward the entry cap.
- Render header bytes in error messages via an escaping describer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o
@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9cb942bb-0f0a-4530-a230-ef0a39a53fb2


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PrzemekGalarowicz
PrzemekGalarowicz merged commit abb274a into main Sep 24, 2026
12 checks passed
@PrzemekGalarowicz
PrzemekGalarowicz deleted the claude/optimistic-heisenberg-8rw8ke branch September 24, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants