Skip to content

fix(bwrap): resolve symlinks and check overlap of mount sources - #34

Closed
Stuart B. Wilkins (stuwilkins) wants to merge 1 commit into
NSLS2:mainfrom
stuwilkins:fix/bwrap-n2c-overlap
Closed

Stuart B. Wilkins (stuwilkins) wants to merge 1 commit into
NSLS2:mainfrom
stuwilkins:fix/bwrap-n2c-overlap

Conversation

@stuwilkins

Copy link
Copy Markdown
Contributor

Summary

Make the last-resort check that keeps the N2C session directory out of the
sandbox catch symlinked mount sources and mounts that lie inside it. Follow-up
to #32.

Description

verify_n2c_state_not_exposed runs after every mount is built and stops the
launch if a bind mount would expose the N2C session directory (it holds the
MSAL cache with the refresh token). It had two gaps:

  • It compared the mount source as written. A source that is a symlink to a
    directory holding the session directory was not caught, for example ~/.npm
    linked to a directory that contains N2C_STATE_DIR.
  • It only tested "the source contains the session directory". A mount whose
    source lies inside the session directory was not caught.

Now each source is compared as written and with symlinks resolved
(readlink -f) against the session directory as written and resolved, in both
directions: the source is the session directory, contains it or lies inside it.
The error message is unchanged.

docs/bw-wrappers.md describes the wider check, with the symlinked ~/.npm
example.

Testing

  • pre-commit run --files lib/bwrap_sandbox_lib.sh docs/bw-wrappers.md: all
    hooks pass (shfmt, shellcheck, codespell, markdownlint, gitleaks, mypy).
  • bash -n lib/bwrap_sandbox_lib.sh is clean.
  • Not yet run with --dry-run against a symlinked source or a mount inside the
    session directory.

Checklist

  • Edit the sections above with relevant information
  • Update in-repo documentation, if applicable
  • Run pixi run -e dev lint locally (or wait for CI) — see .github/workflows/lint.yml
  • Test the changes on a representative managed host (or runansible deployment)

The check for mounts that expose the N2C session directory compared
the source as written, so a symlinked source that leads to a directory
holding it was missed. Compare the source as written and resolved
against the session directory as written and resolved, in both
directions (contains or lies inside).

Assisted-by: opencode:claude-sonnet-5-5
Copilot AI balanced review requested due to automatic review settings October 30, 2026 09:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the expanded session-directory mount guard changes.

No open issues 🎉

What changed in this PR

Strengthens the shared Bubblewrap launcher guard to prevent bind mounts from exposing the N2C session directory and its refresh token, closing two gaps left after #32.

Changes:

  • Checks mount sources and the session directory using both original and symlink-resolved paths.
  • Rejects sources that equal, contain, or lie inside the session directory.
  • Documents the expanded protection, including a symlinked cache example.
File Description
lib/​bwrap_sandbox_lib.sh Adds symlink resolution and bidirectional overlap checks to the final mount guard.
docs/​bw-wrappers.md Explains the expanded session-directory protection and symlink example.

Review effort: Balanced

@stuwilkins

Copy link
Copy Markdown
Contributor Author

Folded into #35, which carries the same commit (47f1865).

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