Skip to content

Bind staging and publication to approved directory identities across platforms #2317

Open

Description

@coderabbitai

Summary

Track the accepted, unfixed ancestor-directory swap race discussed in PR #1918. The requester, @easonLiangWorldedtech, asked to address this through a follow-up change to the shared filesystem primitive, rather than expanding #1918.

A canonical pathname is not a bound directory identity. A local actor who can replace an ancestor directory can redirect a later pathname-based filesystem operation after validation.

Affected areas

  • src/utils/safeWriteJson.ts: final staging-directory validation and _streamDataToFile, which opens the staging file through fsSync.createWriteStream.
  • src/services/file-safety/safeWriteText.ts: staging validation and publication through fs.rename.
  • Their direct regression suites: src/utils/__tests__/safeWriteJson.lockKey.spec.ts, src/utils/__tests__/safeWriteJson.test.ts, and src/services/file-safety/__tests__/safeWriteText.spec.ts.

Remaining failure modes

  1. Staging: After the final fs.realpath(approvedStagingDirectory) succeeds but before the staging file opens, an actor replaces an ancestor with an outside-pointing symlink. The staging open re-resolves the pathname and can write JSON outside the approved directory. Later rejection or cleanup does not undo that disclosure.
  2. Publication: An ancestor swap after final target validation but before the commit rename can redirect pathname-based publication. Canonicalization and advisory locking do not bind the directory identity used by the filesystem operation.

The frozen-parent check detects swaps that occur before that check. It does not close the interval between the check and the subsequent open. Do not treat another pathname check as an atomic solution.

Required changes

  • Design and implement a shared filesystem primitive that binds both staging creation and commit publication to the approved directory identity, using directory-handle-relative operations or an equivalent mechanism with demonstrated guarantees.
  • Route JSON staging and text publication through that primitive without reopening the security boundary through an unbound pathname.
  • Make an explicit maintainer decision on Windows support and fallback behavior. Do not silently claim equivalent confinement on platforms where the mechanism is unavailable.
  • Preserve existing advisory-lock coordination, streaming, atomic publication, cleanup, and durability contracts.
  • Document the supported threat model and any remaining platform limitations.

A POSIX-only final-component O_NOFOLLOW flag is not sufficient for an ancestor-directory swap. The requester reports that Node v25.9.0 on Windows exposes neither fs.openat nor O_NOFOLLOW/O_DIRECTORY constants. Treat this report as design input and verify supported-platform capabilities during implementation.

Acceptance criteria

  • Add a deterministic filesystem regression that swaps the approved parent or an ancestor after the final realpath succeeds and before the staging file opens. Assert that no outside file is created or changed, including temporary files.
  • Add a deterministic regression for an ancestor swap after final publication validation and before rename. Assert that no outside file is created or changed.
  • Exercise swaps involving the scope root itself, so re-resolving a live scope cannot silently move the authorization boundary.
  • Verify the chosen Windows policy and the supported POSIX behavior. Unsupported guarantees must fail explicitly or follow a documented, maintainer-approved policy.
  • Keep the security assertions intact. Land the regressions with the primitive that makes them pass; do not weaken assertions to fit the current implementation.
  • Verify lock release and resource cleanup on rejected operations.

Context

This issue tracks follow-up work; it does not establish that the vulnerability is fixed or that PR #1918 is cleared for merge. The requester explicitly leaves the accepted finding open.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions