Skip to content

feat(path): resolve raw paths through existing prefixes - #349

Merged
steipete merged 6 commits into
mainfrom
codex/path-prefix-20260913
Sep 16, 2026
Merged

steipete merged 6 commits into
mainfrom
codex/path-prefix-20260913

Conversation

@steipete

@steipete steipete commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Add resolvePathPrefixSync to resolve raw paths through existing filesystem prefixes while retaining the unresolved suffix. The helper preserves physical symlink traversal, rejects non-directory traversal, and propagates ambiguous resolution errors. Its result is an observation, not filesystem authorization.

Windows input handling preserves drive-relative and current-drive-root-relative forms without erasing raw dot/parent traversal. Valid namespace drive roots are adapted at filesystem dispatch, including exact roots and immediately missing children. Guarded Root file opens retain their existing stream-alias rejection.

The package builds, all 82 focused prefix, raw-path, and Root-boundary tests pass (21 platform skips), and docs/package validation passes. Fresh independent review through P2 is clean. Complete platform CI passed on the final PR head.

@steipete
steipete requested a review from a team as a code owner September 14, 2026 04:55
@clawsweeper

clawsweeper Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 14, 2026
@clawsweeper

clawsweeper Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed September 16, 2026, 9:43 AM ET / 13:43 UTC (Revision 8).

ClawSweeper review

What this changes

Adds physical path-prefix resolution and Windows path anchoring, alongside ZIP metadata validation, compressed-read cancellation checks, and sibling-filename hardening, with documentation, benchmarks, and tests.

Merge readiness

⛔ Blocked before merge - 5 items remain

The resolver remains a distinct capability absent from main and v0.12.0. No concrete introduced correctness defect was found, but the expanded branch has not satisfied its recorded proof and performance conditions.

Priority: P2
Reviewed head: 6bbb13d4c7ac4e904845b7eb2051e6c1e757e5c0

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch has substantial focused coverage and no established correctness finding, but real-behavior coverage remains incomplete for its expanded scope.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Preserve the previously accepted macOS resolver proof and cancellation correctness evidence. The expanded branch still needs the recorded public-API evidence for ZIP rejection before callbacks/member effects and safe fallback behavior through ordinary/private publication; focused tests and prefix package smoke do not establish those scenarios. Provide redacted terminal output or logs; screenshots/video also count when they show the behavior. Remove private paths, endpoints, addresses, and credentials. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Preserve the previously accepted macOS resolver proof and cancellation correctness evidence. The expanded branch still needs the recorded public-API evidence for ZIP rejection before callbacks/member effects and safe fallback behavior through ordinary/private publication; focused tests and prefix package smoke do not establish those scenarios. Provide redacted terminal output or logs; screenshots/video also count when they show the behavior. Remove private paths, endpoints, addresses, and credentials. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 10 items Applicable repository policy: Read the complete root AGENTS.md and CONTRIBUTING.md. No nested AGENTS.md or maintainer-notes directory was found. Applied filesystem-boundary, compatibility, focused-scope, and public-contract guidance; no target code, tests, or autoreview helpers were executed.
Distinct resolver behavior: The new resolver preserves unresolved dot/parent components, expands physical symlink targets, checks search permission before collapsing parent traversal, and propagates errors other than component ENOENT. Existing ancestor helpers normalize inputs and can fall back after canonicalization failure, so they do not provide the same contract.
Main and release necessity check: Inspected advanced exports at fetched main and v0.12.0: neither exports the proposed resolver. The supplied related PRs remain open or closed unmerged; none establishes a merged replacement for this capability.
Findings None None.
Security None None.

How this fits together

fs-safe supplies filesystem primitives to Node.js applications handling untrusted paths. The resolver returns an existing physical prefix and unresolved suffix; the accompanying archive and staging changes validate inputs before member selection or file publication.

flowchart TD
  A[Raw filesystem path] --> B[Physical prefix traversal]
  B --> C[Existing prefix and raw suffix]
  D[Archive bytes] --> E[Name and kind admission]
  E --> F[Member selection or guarded extraction]
  G[Producer filename] --> H[Validated staging and publication]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Preserve the previously accepted macOS resolver proof and cancellation correctness evidence. The expanded branch still needs the recorded public-API evidence for ZIP rejection before callbacks/member effects and safe fallback behavior through ordinary/private publication; focused tests and prefix package smoke do not establish those scenarios. Provide redacted terminal output or logs; screenshots/video also count when they show the behavior. Remove private paths, endpoints, addresses, and credentials. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - Previously accepted ZIP inputs can now fail with ArchiveFormatError when portable decoder metadata disagrees; affected callers may need to regenerate archives or select a supported backend.
  • Resolve merge risk (P2) - Fallback strings previously returned verbatim can change through sanitization or replacement; the recorded conditional acceptance still requires safe-caller and publication evidence.
  • Resolve merge risk (P1) - Archive performance studies recorded failures alongside unstable controls; the current candidate has no inspected evidence resolving the declared performance conditions.
  • Complete next step (P2) - Complete the outstanding ZIP and fallback consumer proof, satisfy or explicitly resolve the recorded performance holds, and coordinate the prerequisite PRs before landing.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test growth Production net +260 lines; tests net +1,167 lines Counts include TypeScript and Rust test sections, excluding benchmarks/docs/scripts; production growth implements the resolver and three separately discussed hardening concerns.
Stacked scope 50 files, 6 commits The introduced delta includes three open prerequisite changes beyond the advertised resolver.

Merge-risk options

Maintainer options:

  1. Satisfy prerequisite landing conditions (recommended)
    Complete the recorded consumer proof and comparative performance requirements before landing the included archive and filename changes.
  2. Keep the resolver queued separately
    Pause this stacked branch while its prerequisite PRs are resolved, then refresh the remaining resolver-only delta.

Technical review

Best possible solution:

Land independently validated archive and filename prerequisites, then retain the additive observational resolver with explicit compatibility documentation and fulfilled consumer-proof conditions.

Do we have a high-confidence way to reproduce the issue?

Not applicable to the central feature request; source inspection establishes that existing ancestor helpers do not preserve the requested raw suffix semantics. No introduced defect was reproduced.

Is this the best way to solve the issue?

Yes for the resolver itself: an additive observation API preserves existing helpers' contracts. The complete stacked branch still depends on satisfying the separately recorded compatibility and performance conditions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 2374204cc8d0.

Labels

Label justifications:

  • P2: This is a useful bounded filesystem capability with hardening prerequisites and no established urgent current-user outage.
  • merge-risk: 🚨 compatibility: The branch changes accepted ZIP interpretations and exported fallback-name behavior, with consumer compatibility proof still outstanding.
  • merge-risk: 🚨 other: Explicit comparative performance conditions remain unresolved and are not settled by ordinary green CI.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Preserve the previously accepted macOS resolver proof and cancellation correctness evidence. The expanded branch still needs the recorded public-API evidence for ZIP rejection before callbacks/member effects and safe fallback behavior through ordinary/private publication; focused tests and prefix package smoke do not establish those scenarios. Provide redacted terminal output or logs; screenshots/video also count when they show the behavior. Remove private paths, endpoints, addresses, and credentials. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Applicable repository policy: Read the complete root AGENTS.md and CONTRIBUTING.md. No nested AGENTS.md or maintainer-notes directory was found. Applied filesystem-boundary, compatibility, focused-scope, and public-contract guidance; no target code, tests, or autoreview helpers were executed. (AGENTS.md:23, 6bbb13d4c7ac)
  • Distinct resolver behavior: The new resolver preserves unresolved dot/parent components, expands physical symlink targets, checks search permission before collapsing parent traversal, and propagates errors other than component ENOENT. Existing ancestor helpers normalize inputs and can fall back after canonicalization failure, so they do not provide the same contract. (src/path-prefix.ts:30, 6bbb13d4c7ac)
  • Main and release necessity check: Inspected advanced exports at fetched main and v0.12.0: neither exports the proposed resolver. The supplied related PRs remain open or closed unmerged; none establishes a merged replacement for this capability. (src/advanced.ts:7, 2374204cc8d0)
  • Release source inspection: The latest supplied release retains the existing ancestor helper without resolvePathPrefixSync. (src/advanced.ts:7, 914cd7b41388)
  • Introduced compatibility boundary: Portable ZIP loading now rejects decoder name/kind disagreement; docs explicitly identify unsupported UNIX-only directories, backslash-only directories, and unexposed non-UNIX symlink metadata. Native member reads validate the complete manifest before selecting content. (src/archive-zip-loader.ts:52, 6bbb13d4c7ac)
  • Conditional fallback acceptance: The discussion at fix(security): sanitize sibling callback names #382 (comment) explicitly accepts fallback hardening conditionally on safe-caller compatibility, ordinary/private publication, rejection-before-producer proof, and comparative performance evidence. This branch contains that prerequisite at 5d4ebba. (src/sibling-staged-file.ts:190, 5d4ebba092cd)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Complete ZIP rejection-before-effects and safe fallback publication evidence while retaining accepted historical proof.
  • Resolve the recorded comparative performance conditions with candidate evidence or an explicit authorized disposition.
  • Coordinate prerequisite landing and refresh the resolver's remaining scope.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (7 earlier review cycles)
  • reviewed 2026-09-14T05:00:02.186Z sha 3e9fc77 :: needs changes before merge. :: [P2] Preserve search-permission checks before collapsing parent traversal
  • reviewed 2026-09-14T05:25:33.976Z sha f97dc77 :: blocked before merge. :: none
  • reviewed 2026-09-14T05:36:03.914Z sha f080834 :: blocked before merge. :: none
  • reviewed 2026-09-14T05:45:57.912Z sha ba5f10b :: needs maintainer review before merge. :: none
  • reviewed 2026-09-15T22:50:51.318Z sha 7a9281a :: needs maintainer review before merge. :: none
  • reviewed 2026-09-15T23:11:04.821Z sha af9b7ab :: needs maintainer review before merge. :: none
  • reviewed 2026-09-16T13:30:52.572Z sha ccbb42e :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 14, 2026
@steipete
steipete force-pushed the codex/path-prefix-20260913 branch from f97dc77 to f080834 Compare September 14, 2026 05:27
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Sep 14, 2026
Bind ZIP physical entry names and kinds to decoder metadata before callbacks or member selection, rejecting ambiguous interpretations consistently.
@steipete
steipete force-pushed the codex/path-prefix-20260913 branch from af9b7ab to ccbb42e Compare September 16, 2026 13:25
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 16, 2026
Stop zstd and bzip2 compressed-input refills after cancellation while retaining decoded-output and complete archive validation.
Sanitize fallback filenames and validate completed sibling callback components before producers run, retaining bounded names and Windows device-name rejection.
Add resolvePathPrefixSync for physical traversal through existing prefixes with a raw missing suffix, preserving dangling aliases and propagating ambiguous resolution failures.
@steipete
steipete force-pushed the codex/path-prefix-20260913 branch from ccbb42e to 6bbb13d Compare September 16, 2026 13:38
@steipete
steipete merged commit 6bbb13d into main Sep 16, 2026
33 checks passed
@vincentkoc
vincentkoc deleted the codex/path-prefix-20260913 branch September 25, 2026 11:15

This branch was successfully deployed

1 active deployment
github-pages — 6bbb13d4 Deployed Sep 16, 2026 by steipete via Deploy docs #282
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant