Invalidate path-restriction cache when symlink realpath changes - #756
Conversation
a3840b7 to
5728006
Compare
TheGreatAxios
left a comment
There was a problem hiding this comment.
Review · Comment
The path gate revalidates cached verdicts against the nearest-existing realpath before reusing them.
Findings
CONTRIBUTING.md:100— Commit57280066contains five body lines over the mandatory 72-character limit;git log origin/main..HEAD --format="%b" | awk "length > 72"returns each one. Could you rewrap the commit body so the branch history meets the repository rules?CONTRIBUTING.md:138— The PR body usesProblem,Fix, andTestsections instead of the requiredSummaryandVerificationshape. Could you update it so the PR metadata follows the source-of-truth template?
Notes
- These findings should be resolved before merge; GitHub does not allow the PR author account to request changes on its own PR.
bun run checkexits 0 locally; GitHub build, lint, formatting, typecheck, and CLA checks are green.
isRestricted cached verdicts only by lexical absolute path. After a workspace symlink changed from inside to outside, the stale unrestricted verdict persisted and corrupted gate ask-vs-auto decisions. The cache stores the resolved path with each verdict and validates it on every call. realpathNearestOr resolves symlinks through the nearest existing ancestor, including for nonexistent targets.
5728006 to
1897f0c
Compare
TheGreatAxios
left a comment
There was a problem hiding this comment.
Review · Comment
The path-restriction cache invalidation branch now meets the repository metadata rules.
No findings.
Notes
- Commit
1897f0c7wraps every subject and body line within 72 characters. - The PR description uses the required
SummaryandVerificationsections. bun run checkexits 0 against the amended commit.
TheGreatAxios
left a comment
There was a problem hiding this comment.
Review · Blocking
The path restriction cache invalidates entries when a symlink's resolved target changes.
Finding
src/permission/path-restriction.ts:204— The cache recordscurrentRealpathfrom one filesystem snapshot but pairs it with a verdict computed by a second resolution at line 219. If the symlink retargets between those calls, the cache can associate an unrestricted verdict with an outside realpath and reuse it later. Could we compute and cache the verdict from one resolved snapshot so the security decision and key cannot diverge?
GitHub CI is green, but this time-of-check/time-of-use window affects the permission boundary and should be resolved before merge.
TheGreatAxios
left a comment
There was a problem hiding this comment.
Review · No findings
The cache now derives validation, permission verdict, and storage from one resolved-path snapshot.
The deterministic retarget race and existing inside-to-outside regression pass, bun run check exits 0 with 5,897 tests, all required GitHub checks pass, and the prior blocking finding is resolved.
Summary
Verification
bun run checkFixes CL-6708