Repository navigation
MNT: script cleanups - #25
Merged
Stuart B. Wilkins (stuwilkins) merged 40 commits intoOct 20, 2026
Merged
Conversation
Pin actions/checkout from the floating @v7 tag to the specific commit SHA 3d3c42e (v7.0.1) for supply-chain reproducibility, consistent with the approach already used for the prefix-dev/setup-pixi action in the same workflow. Assisted-by: opencode:claude-sonnet-4-6
- On push, $BEFORE is 40 zeros on branch creation and may be unfetchable
after a force-push; both break `pre-commit --from-ref`. Fall back to
the --all-files path when $BEFORE is all-zeros or `git cat-file -e`
can't find it.
- Pass environments: dev to setup-pixi and drop the separate
`pixi install -e dev` step (one fewer solve/install).
- Generate the job-summary version table at runtime from `pixi list -e
dev --json` and the .pre-commit-config.yaml rev tags instead of
hardcoding versions that silently drifted from the lockfile (mypy was
already pinned as "*" in pixi.toml yet listed as a fixed version).
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
decrypt_env_file read required-variable values through a bash `local -n`
nameref, which does not exist in zsh: sourced from a zsh startup file it
failed with "local: bad option: -n" and then falsely reported every
required variable empty (return 2) even when the decrypt had set it.
Read the value by indirection through `eval "value=\${$name:-}"` instead
(the portable idiom this file used before the nameref was introduced),
guarded so it is still safe:
- reject a name starting with the reserved __gpgpw_ prefix (would read one
of this function's own locals), and
- reject a name that is not a plain shell identifier (would let the eval
inject code),
both via pure glob `case` patterns that behave identically in bash and zsh.
Verified in bash and zsh: valid vars pass, empty required var returns 2,
an injection-shaped name and a reserved name both return 2, and the
decrypted-content eval-failure path still returns 4.
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
decrypt_env_file ran eval on the decrypted env file without checking its status, so a syntax error or failing command was silently swallowed (the function could still return 0). Capture eval's status directly (inside `if ! eval ...` the negated pipeline would make $? always 0) and return a new documented code 4 with the underlying exit code in the message. Assisted-by: ClaudeCode:claude-fable-5 Assisted-by: ClaudeCode:claude-sonnet-5
…inate ruleset lookup User-supplied values (--approvers slug, --exclude-branches patterns) were interpolated raw into JSON payloads and API URLs and validated only after steps 1-4 had already mutated the repo. Validate them against a safe charset in the flag block before any API call, and rebuild every payload carrying variable input with 'jq -n --arg/--argjson/--args' so the injection class is gone (JSON shapes verified identical). The ruleset existence GET was unpaginated (a match past page 1 was missed, causing a duplicate POST) and swallowed transient errors as 'not found'. Add --paginate, collapse duplicate-name matches with first(...), abort on a real API error, and guard against a newline-joined id. Also: reject extra positional args, send error-path help to stderr, and declare _pats/_refs local. Assisted-by: OpenCode:claude-fable-5 Assisted-by: ClaudeCode:claude-opus-4-8
- Query rulesets with includes_parents=false: the list endpoint includes
org-inherited rulesets by default, so an org-level ruleset sharing our
standard name could be selected by the name-only jq filter and the
subsequent PUT repos/{repo}/rulesets/{org-id} would 404 mid-run.
- When both the full-protection and creation-restriction rulesets are
needed in one invocation, fetch the ruleset list once and extract both
IDs from the cached stream (jq -s) instead of two --paginate GETs; a
fetched flag (not string emptiness) marks the cache valid so a repo
with no rulesets yet does not re-fetch.
- Drop the unreachable re-validation in _build_exclude_json: the
pre-flight loop already rejects empty/invalid patterns under exactly
the call site's condition.
- docs: the "non-default branch" example passed the default branch.
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
…g, and idempotency OData filter values were URL-encoded but not OData-escaped, so an apostrophe in --name broke every run; add _odata_str() (doubles ') at every filter site. Retry policy retried permanent 400s (~150s) while never retrying 429/5xx: retry (404,429,500,502,503,504), honour Retry-After, and retry 400 only on SP-replication-lag messages. Robustness: add a 30s urlopen timeout and catch URLError/TimeoutError/ JSONDecodeError as exit 4 (were uncaught tracebacks). SP creation and the grants POST now tolerate create-vs-get races (graph_or_exists / swallow_exists). Grant lookup filters consentType eq 'AllPrincipals' and adminConsentGranted is derived from reconciled state; the JSON output reports the merged redirect URIs; graph_list follows @odata.nextLink; -r values are deduped; non-404 owner-lookup failures exit 4; argparse usage errors exit 1 (2 is reserved for az-missing). Cleanups: drop the redundant az-account-show probe and the dead expect_no_content param; skip the requiredResourceAccess read-back when unchanged; add graph_json() to remove five type:ignore; move imports to top; urlencode the query string; teach _status_tag the 'added' status. Assisted-by: OpenCode:claude-fable-5 Assisted-by: ClaudeCode:claude-opus-4-8
The delegated-grant POST discarded graph_with_retry's already_existed
flag and unconditionally reported delegated_grant=created and
adminConsentGranted=true. A grant is keyed only on
(clientId, resourceId, consentType), so a pre-existing AllPrincipals grant
with a scope subset (portal-created, or replication-lagged behind the
initial lookup) was reported as fully consented while scopes like
"openid email profile offline_access" were never granted — OIDC logins
then fail with AADSTS65001 despite the tool reporting success.
- When the swallow fires, re-list the grant (with linear-backoff retry,
since Graph documents replication lag on this endpoint) and run the
same scope union/PATCH reconciliation as the existing-grant branch;
report existed/updated truthfully.
- Give SP creation and both owner $ref POSTs retry via
graph_with_retry(swallow_exists=True): they act on seconds-old objects
and were the calls most exposed to the ~60s propagation lag the retry
helper was written for. graph_or_exists is now unused and removed.
- Add the "does not reference a valid application" appId-propagation
message to _is_sp_propagation_error.
- Escape "/" in the --owner UPN (quote(safe="")) so a slash cannot
extend the URL path to a different Graph endpoint.
- Drop the dead final_scopes/issubset bookkeeping.
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
…enssl DN parse The INT/TERM/HUP trap ran cleanup and returned 0, so Ctrl-C let the script resume and exit ~0 with helpers killed. Keep cleanup on EXIT and re-raise each fatal signal (clear the signal+EXIT traps, then kill -SIG $$) so the process dies with 128+N and cleanup runs once (verified: Ctrl-C exits 130). An unreadable input file returned 3, which the docs define as 'openssl parse error'; reserve 3 for real parse failures and map an unreadable file to the usage-error code 1 (header + docs updated). The verify-failure reason no longer claims 'signature mismatch' for every openssl verify failure. Perf: parse subject+issuer in one 'openssl x509 -subject -issuer' call and cache per cert so the --verify loop reuses them instead of re-forking. Also drop a redundant QUIET test and collapse the two-sed usage(). Assisted-by: OpenCode:claude-fable-5 Assisted-by: ClaudeCode:claude-opus-4-8
…olour
- ensure_tmpdir now checks mktemp -d: a failing TMPDIR previously left
TMPDIR_ empty and cascaded into misleading per-link "FAIL (signature
verification failed)" (exit 4); it now reports the real problem and
exits with a new documented operational-error code 5.
- cert_info extracts subject, issuer, and signature algorithm from one
openssl x509 invocation instead of two per certificate; output is
byte-identical (verified against the previous version on a generated
CA-signed chain, with and without --verify).
- Document the --no-colour alias in --help and docs.
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
- finalize_sandbox_path deduplicates the dynamically-discovered bin
directories against each other and against the base SANDBOX_PATH, so the
same dir is not appended twice.
- build_binary_masks canonicalizes and dedupes the search prefixes once
(on merged-/usr systems /bin, /sbin, /usr/bin, /usr/sbin resolve to the
same dir) and resolves every candidate with a single readlink -f fork
instead of one realpath fork per binary.
- build_dynamic_tool_mounts preloads ${_HOME}/bin into _MOUNTED_PREFIXES so
later mount attempts under it are de-duplicated.
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
Candidate mount paths are canonicalized before the safety checks, but the
$HOME-or-ancestor test compared them only against the logical $HOME. On
hosts where home is reached through a symlink (NFS/automount), a canonical
candidate such as the real home directory slipped through (e.g.
PIXI_HOME=<real home> was accepted and the whole home tree mounted).
- resolve_common_paths records _HOME_CANON (readlink -f $HOME), and
_bwrap_path_blocked checks the ancestor-of-home condition against both
the logical and the canonical home. The blocked list itself was already
matched at symlink targets by _bwrap_load_blocked_prefixes.
- _PWD is the physical working directory (pwd -P), and build_workdir_mount
canonicalizes _BIND_DIR before _check_path_safe. A non-git working
directory reached through a symlink into ~/.ssh was previously bound
read-write; it is now refused. PWD in the sandbox now agrees with the
physical bind (git rev-parse already returned a physical root).
- _mount_ro_dir_if_needed judges safety on the canonical directory while
still mounting the logical one, so a symlinked on-PATH tool directory
(e.g. ~/bin2 -> ~/.ssh) is refused for every caller.
- _emit_home_intermediate_dirs recognises paths under the canonical home.
- USER/LOGNAME come from _USER (${USER:-$(bwrap_account_name)}), so the
launchers no longer fail under `set -u` when $USER is unset (cron/CI).
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
Assisted-by: OpenCode:claude-opus-5-5
- Tilde expansion is centralized in _expand_leading_tilde, which handles
~, ~/path and ~user/path (via getent), for git include paths and
--ro-path/--rw-path. The old ${v/#\~/${_HOME}} substitution corrupted
"~alice/foo" into "${HOME}alice/foo"; an unresolvable ~user is now
skipped (git include) or rejected (--ro-path/--rw-path).
- parse_git_includes tracks the current section and only honours
`path =` inside [include] and [includeIf "..."] (case-insensitive,
quoted subsections, key on the header line), so a `path =` key in an
unrelated section such as [difftool "x"] is no longer recorded as an
include -- which mounted it, or aborted the launcher when it pointed
into a blocked location.
- _resolve_npm_prefix replaces the echo-based _get_npm_prefix. It was
always called through $(...), so its cache was set in a subshell and
lost; npm ran on every lookup and wrote ~/.npm/_logs on the host even
under --dry-run. The cache now persists and npm_config_cache points at
a throwaway directory for the lookup.
- The on-PATH directory of an npm-symlinked tool is mounted as well as the
npm prefix, so the path the tool is invoked by exists in the sandbox.
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: OpenCode:claude-opus-5-5
build_extra_path_mounts emitted every --ro-bind before every --bind, so
`--rw-path DIR --ro-path DIR/sub` left DIR/sub silently WRITABLE (the later
parent --bind covered it). Read-only directories also went through the
_MOUNTED_PREFIXES dedup, which silently dropped any --ro-path under a
pre-registered prefix such as /tmp (a tmpfs in the sandbox, so the path was
simply absent) and a read-only subdirectory of the working directory.
Emit the user paths ancestors-first across both modes instead: bwrap
applies binds in order and the last one covering a mount point wins, so a
nested child of either mode overrides its parent. Both
`--rw-path DIR --ro-path DIR/sub` and the reverse (read-only tree with one
writable subdir) now work.
- Validate every path up front, propagating validate_extra_path's exit
from the command substitution explicitly; reject a path given as both
--ro-path and --rw-path.
- Sort the combined set with LC_ALL=C (an ancestor is a byte prefix of
its descendants) and emit user paths directly, bypassing the dedup.
- Skip a path's intermediate --dir entries when an ancestor is already
bind-mounted (a --dir there would clobber the bind); prior bind
destinations are collected by walking BWRAP_ARGS.
- _bwrap_flag_arity sets _BWRAP_FLAG_ARITY instead of echoing, so the
BWRAP_ARGS walks here and in print_dry_run do not fork per argument.
- build_workdir_mount registers the working-directory bind in
_MOUNTED_PREFIXES, so a tool resolved from inside the repo
(node_modules/.bin, .pixi/envs/*/bin) no longer gets an --ro-bind on top
of the read-write bind (EROFS on npm/pixi install).
- Document the nesting in every launcher's help and docs/bw-wrappers.md.
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
Assisted-by: OpenCode:claude-opus-5-5
print_dry_run wrapped bwrap argument values in bare single quotes
('%s'), so a path containing a single quote produced a command that could
not be pasted back into a shell. Quote flags and values with %q instead;
the REDACTED placeholder for secret-looking --setenv values is unchanged.
Verified by re-parsing the printed command with eval: for all five
launchers the argument vectors are identical to the previous output, and a
--ro-path containing ', " and spaces now round-trips exactly.
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
Assisted-by: OpenCode:claude-opus-5-5
--dry-run is documented as printing the bwrap command without running it,
but every launcher still created directories and files on the host first:
ensure_host_dirs (mkdir/touch), the --init-auth stub files (opencode and
codex auth.json, claude settings.json and .credentials.json, copilot
config.json), bwclaude's ~/.claude.json, and the per-repository
project-dotfiles directory of bwopencode/bwopencode2. A --dry-run against
a fresh $HOME created 9-18 host paths per launcher; it now creates none.
- Skip ensure_host_dirs and every stub/mkdir under --dry-run, in all five
launchers.
- Keep the printed command faithful to a real run: binds whose source a
real run would have created are still emitted. The new library helper
_persist_bind_wanted covers the --init-auth stubs, bwclaude always
binds its state file, and the project-dotfiles bind is unconditional.
- Create bwclaude's --init-auth settings.json stub with umask 077 (was
022, world-readable).
Assisted-by: opencode:claude-sonnet-4-6
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: OpenCode:claude-opus-5-5
…docs bwclaude: ensure_host_dirs created ~/.claude/plans on the host, but the persistence loop in build_claude_home_mount never bound it, so plan-mode plans written inside the sandbox landed on the ~/.claude tmpfs and were silently discarded while an empty directory sat on the host. Bind plans/ read-write like the other persisted directories (as bwopencode already does), so plans survive the session and are shared with unsandboxed claude. Plan files are plain markdown, not configuration that runs outside the sandbox. bwopencode: the file header and the build_opencode_data_mounts comment described plans/ as ephemeral, but the persist loop has always included it. Update both, and the persisted list in docs/bw-wrappers.md. docs: list what bwclaude persists under ~/.claude. Assisted-by: opencode:claude-sonnet-4-6 Assisted-by: OpenCode:claude-opus-5-5
The default (non --persist-config) path staged config.toml to a mktemp
file and registered a trap to delete it on EXIT. However, launch_sandbox
uses exec to replace the shell process with bwrap, and bash does NOT run
EXIT traps across exec. The trap fired only on the --dry-run path
(print_dry_run calls exit 0) or if a signal arrived during setup, leaving a
copy of the user's config.toml in $TMPDIR on every normal run. The
INT/TERM trap also ran rm -f without re-raising the signal.
- Open the staged copy as an fd immediately after writing it, then unlink
the temp file before exec. The kernel keeps the data alive via the open
fd; bwrap receives it via --file and creates a plain writable file in
the sandbox tmpfs from it. No trap and no cleanup are needed.
- Copy with plain cp instead of cp -p: -p re-applied the source's mode
(typically 0644) over mktemp's 0600, leaving a world-readable copy in
the shared temp directory until the unlink.
- Under --dry-run, stage nothing and print a <config-fd> placeholder: a
real fd number could not be reproduced by copy-paste.
- Update the file-header comment to describe the fd-based strategy.
Verified with a real run: the sandbox sees the config and can append to it,
the host file is unchanged, and no bwcodex-config.*.toml is left in $TMPDIR
(main leaves one per run).
Assisted-by: opencode:claude-sonnet-4-6
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: OpenCode:claude-opus-5-5
…guards bwcodex NSLS2#3: build_codex_env called pass_through_if_set for CODEX_HOME, CODEX_SQLITE_HOME, CODEX_API_KEY, CODEX_ACCESS_TOKEN, and CODEX_CA_CERTIFICATE individually and then emitted each of them a second time via the generic CODEX_* while-read sweep. The duplicates are harmless (bwrap last-write wins, same value) but noisy and misleading. Remove the five redundant explicit calls; only SSL_CERT_FILE and RUST_LOG genuinely need the explicit lines because they do not match the CODEX_* prefix. bwcopilot NSLS2#2: the skills/ and agents/ bind loop guarded each path with `[[ -e "${_path}" ]]`, but ensure_host_dirs unconditionally creates both directories, making the guards always true. Remove the dead existence checks and bind unconditionally. On --dry-run (where ensure_host_dirs is now skipped) the bind args are still emitted so the dry-run output accurately reflects the real invocation. Assisted-by: opencode:claude-sonnet-4-6
…weeps
The GH_TOKEN_* passthrough block was duplicated verbatim in all five
launchers, each with its own --dry-run REDACTED substitution, and similar
prefix sweeps existed for CLAUDE_*, CODEX_* and COPILOT_PROVIDER_*.
- Add forward_env_by_prefix PREFIX to the library: iterate `compgen -e`
with `while IFS= read -r` and append a --setenv per matching variable.
- Use it for GH_TOKEN_ (all five launchers), CLAUDE_ (bwclaude), CODEX_
(bwcodex) and COPILOT_PROVIDER_ (bwcopilot). This also replaces
bwcopilot's word-splitting `for _key in $(compgen -e)` loop.
- Drop the per-launcher REDACTED swap: print_dry_run already redacts
every --setenv whose name matches _is_secret_env_name (*TOKEN*), and
the real launch path is unchanged.
- bwcopilot keeps its explicit COPILOT_HOME / COPILOT_CACHE_HOME /
COPILOT_MODEL allowlist, and bwopencode/bwopencode2 keep their
OPENCODE_* allowlists; neither is replaced by a prefix sweep.
No behaviour change: with CLAUDE_*, CODEX_*, COPILOT_PROVIDER_*,
COPILOT_OTHER and GH_TOKEN_* set, the forwarded variables are identical
before and after for every launcher, in --dry-run and in a real run.
Assisted-by: opencode:claude-sonnet-4-6
Assisted-by: OpenCode:claude-opus-5-5
…--debug
- --help: every launcher resolved its tool binary before checking
SHOW_HELP, so `bw* --help` aborted with "not found" / "installed
incorrectly" before printing anything when the tool was missing.
Handle --help first; the new library helper bwrap_show_tool_help then
appends the tool's own help (re-exec with --exec "TOOL --help", as
before) only when the tool is usable, and otherwise says so and exits 0.
- --exec example: `--exec 'env | sort'` cannot work -- CMD is split on
whitespace and run without a shell, so `|` became a literal argument.
Document `--exec CMD [ARGS...]` with `--exec bash -c 'env | sort'`
(arguments after CMD are passed through unchanged) in all five
launchers, docs/bw-wrappers.md and the README.
- bwclaude --debug no longer prepends `--debug --verbose` to an --exec
command (`bwclaude --debug --exec bash` ran `bash --debug --verbose`).
This is the minimal form of the shared-front-end refactor on
mnt/more_cleanup; the parser itself stays per launcher for now.
Assisted-by: opencode:claude-sonnet-4-6
Assisted-by: OpenCode:claude-fable-5
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: OpenCode:claude-opus-5-5
Under --dry-run, bwcopilot still opened ~/.copilot/config.json as a live fd and printed its number (`--file 10 ...`), which cannot be reproduced by copy-paste. Emit a `<config-fd>` placeholder instead, matching bwcodex. A real run is unchanged: the sandbox still receives a copy of config.json. Assisted-by: ClaudeCode:claude-fable-5 Assisted-by: ClaudeCode:claude-sonnet-5 Assisted-by: OpenCode:claude-opus-5-5
SSL_CERT_FILE was forwarded into the sandbox but the file it names was never mounted. A bundle under $HOME (hidden by the home tmpfs), under /tmp (a sandbox tmpfs) or anywhere else outside the mounted system trees was therefore missing inside the sandbox and Codex's TLS failed confusingly. build_ssl_cert_file_mount canonicalizes the path, read-only binds the file at its canonical location and points SSL_CERT_FILE there. When the path is not a readable file, or resolves into a blocked location (checked with the non-exiting _bwrap_path_blocked), it warns and does not forward the variable. Assisted-by: ClaudeCode:claude-fable-5 Assisted-by: ClaudeCode:claude-sonnet-5 Assisted-by: OpenCode:claude-opus-5-5
The wrapper keyed its mounts on CLAUDE_HOME / CLAUDE_CACHE_HOME, which the
Claude CLI does not read; the CLI reads CLAUDE_CONFIG_DIR and
XDG_CACHE_HOME. A host-exported CLAUDE_CONFIG_DIR was forwarded by the
CLAUDE_* sweep while the mounts stayed on ~/.claude, so persistence and
credentials silently broke.
- Key the Claude home on CLAUDE_CONFIG_DIR (default ~/.claude); an
override is canonicalized and safety-checked with
_bwrap_canonicalize_safe. The state file is
$CLAUDE_CONFIG_DIR/.claude.json when the override is set, else
~/.claude.json, and is safety-checked before it is bound read-write.
Derive the cache dir from XDG_CACHE_HOME. CLAUDE_HOME and
CLAUDE_CACHE_HOME are no longer consulted.
- Set CLAUDE_CONFIG_DIR in the sandbox only when the host sets it (to
the canonical directory the mounts use). Otherwise it stays unset, so
sandboxed and unsandboxed claude share ~/.claude and ~/.claude.json.
- settings.json can define hooks/apiKeyHelper/env that later run
UNSANDBOXED on the host; a sandboxed session writing it was a
sandbox-escape channel. Provide it as an ephemeral --file copy by
default (like .credentials.json); bind it read-write only under
--init-auth.
- Require the foundry key and host only when claude itself is launched,
not under --dry-run or --exec (this also lets `bwclaude --help` show
claude's own help without credentials).
- Emit <credentials-fd>/<settings-fd> placeholders under --dry-run
instead of opening live fds whose numbers are unreproducible.
- Document the Claude home, state file and settings.json handling in the
header, --help and docs/bw-wrappers.md.
The header and build_claude_home_mount comments described ~/.claude.json
as "only UI/session state ... not a credential, safe to persist". Per the
Claude Code docs it also holds the OAuth session, per-project trust
decisions and personal MCP servers; correct the comments and docs and mark
the read-write bind as a known gap: a sandboxed session can add an MCP
server or trust decision that unsandboxed claude will honour later.
Tightening that is left for later.
Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-sonnet-5
Assisted-by: OpenCode:claude-opus-5-5
--github-tokens is accepted by all five launchers, but docs/bw-wrappers.md repeated the same row in the bwopencode, bwclaude, bwcopilot and bwcodex sections (and described it separately for bwopencode2), while the common options table omitted it. Move it to the common table, drop the per-wrapper copies (and bwcopilot's now-empty "Additional options" table), and add it, together with --ro-path/--rw-path, to the README's common options table. Assisted-by: ClaudeCode:claude-fable-5 Assisted-by: ClaudeCode:claude-sonnet-5 Assisted-by: OpenCode:claude-opus-5-5
Forwarded credentials (AIFAPIM_API_KEY, ANTHROPIC_FOUNDRY_API_KEY,
CODEX_API_KEY, GH_TOKEN, GH_TOKEN_*, ...) were passed as
`--setenv NAME VALUE` on bwrap's command line. bwrap stays resident for
the whole session, and /proc/<pid>/cmdline is world-readable, so any local
user could read them with `ps auxww`. The --dry-run redaction on main only
covered the printed text.
/proc/<pid>/environ, unlike cmdline, is readable only by the process owner.
We already launch bwrap through `env -i`; dropping --clearenv makes bwrap
forward that curated environment straight into the sandbox. The values
touch only `env`'s own argv for the instant before it execs bwrap.
- New SANDBOX_ENV array of NAME=VALUE entries, appended to the `env -i`
line in launch_sandbox.
- pass_through_if_set and forward_env_by_prefix append to SANDBOX_ENV
instead of BWRAP_ARGS; so do bwclaude's ANTHROPIC_FOUNDRY_API_KEY
(from AIFAPIM_API_KEY) and the OPENCODE_* allowlist loops of bwopencode
and bwopencode2. Non-secret enforced overrides stay as --setenv, which
bwrap applies on top of the inherited environment, so they still win.
- build_env_vars drops --clearenv and the --unsetenv fallback for bwrap
< 0.5 (bwrap's environment is already exactly what env -i gives it);
HAS_CLEARENV is removed. launch_sandbox no longer injects LANG /
LC_CTYPE defaults, which would now have reached the sandbox.
- print_dry_run prints the SANDBOX_ENV entries on the `env -i` line,
with secret-looking values REDACTED as before.
- Drop the stale --clearenv notes from the headers, docs and README, and
document the new channel under "Credentials and secrets handling".
Verified with real bwrap runs: the environment seen inside the sandbox is
identical before and after for all five launchers (with and without LANG
set), and while a sandbox runs no process's cmdline contains the forwarded
secrets (before: the bwrap process did).
Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: OpenCode:claude-opus-5-5
The personal skills directories the tools read were mostly missing from
the sandboxes: $HOME is a tmpfs and only the deployment-managed skills
(/etc/*/skills, /opt/nsls2/skills) were mounted. bwcodex and bwopencode
bound ~/.codex/skills and ~/.config/opencode/skills as one unit, so a
skill symlinked in from a separate repository dangled inside the sandbox.
Add build_personal_skills_mounts DIR... to the library. Each existing DIR
is scanned one level deep and every entry (one skill) is resolved with
readlink -f and bound read-write at its logical path, so the target
repository does not need to be mounted and the agent can update existing
skills (for a symlinked skill, that edits its source repository). Only
existing entries are host-backed: a new top-level skill created inside the
sandbox lands on the tmpfs and is discarded on exit. An entry that resolves into a
blocked location, or is a directory that contains one, is skipped with a
warning; a DIR already covered by an earlier bind is left alone.
- bwclaude: $CLAUDE_CONFIG_DIR/skills (default ~/.claude/skills)
- bwcodex: ~/.codex/skills (replacing its whole-directory bind) and
~/.agents/skills
- bwcopilot: ~/.agents/skills (~/.copilot/skills stays read-write with
the Copilot home)
- bwopencode: ~/.config/opencode/skills (now skipped by
resolve_config_symlinks), ~/.claude/skills, ~/.agents/skills
- bwopencode2: ~/.config/opencode2/opencode/skills (skipped by its
resolve_config_symlinks), ~/.claude/skills, ~/.agents/skills
The deployment-managed skills mounts are unchanged. Documented in the
launcher headers and a new "Personal skills directories" section of
docs/bw-wrappers.md. The bwcodex header list is also reordered so the
/etc/codex entries no longer sit inside the ~/.codex sub-list.
Assisted-by: opencode:claude-sonnet-5
Assisted-by: OpenCode:claude-opus-5-5
Assisted-by: codex:gpt-6-sol
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved mount-security, skills-visibility, and older-Bash compatibility issues need correction.
4 open issues
What changed in this PR
This PR cleans up administrative scripts and strengthens the shared sandbox wrappers’ credential handling, mounts, and persistence.
Changes:
- Refines environment forwarding, personal skills mounts, and dry-run behavior.
- Improves Azure provisioning, branch protection, and certificate-processing utilities.
- Updates documentation and lint workflow reporting.
| File | Description |
|---|---|
| README.md | Updates shared wrapper options. |
| lib/gpg-passwd.sh | Adds portable variable validation and evaluation errors. |
| lib/bwrap_sandbox_lib.sh | Revises environment forwarding and mount handling. |
| docs/pemdecompose.md | Documents options and exit codes. |
| docs/gpg-passwd.md | Documents calling conventions and errors. |
| docs/gh-protect-branch.md | Corrects the non-default branch example. |
| docs/bw-wrappers.md | Explains skills, persistence, and credential handling. |
| bin/pemdecompose | Consolidates certificate parsing and improves cleanup. |
| bin/gh-protect-branch | Validates inputs and improves ruleset handling. |
| bin/bwopencode2 | Adds skills mounts and refines dry runs. |
| bin/bwopencode | Updates skills, persistence, and environment forwarding. |
| bin/bwcopilot | Refines customization mounts and credential staging. |
| bin/bwcodex | Updates package mounts and configuration staging. |
| bin/bwclaude | Aligns configuration paths and isolates settings. |
| bin/azoidcapp | Improves Graph requests and consent reconciliation. |
| .github/workflows/lint.yml | Hardens reference handling and reports tool versions. |
Review effort: Balanced
Assisted-by: opencode:claude-sonnet-5.5
pre-commit was failing in obscure ways locally, moving to better tool was easier than debugging a tool we should move off of anyway. Assisted-by: opencode:claude-sonnet-5.5
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved mount regressions and an undeclared runtime dependency need correction before approval.
7 open issues
Review effort: Balanced
This was referenced Oct 20, 2026
Closed
There was a problem hiding this comment.
🟡 Changes recommended
Mount safety and dependency handling need correction before approval.
7 open issues
1 resolved since last review
Review effort: Balanced
Personal skills were mounted one entry at a time over a tmpfs, so a new
top-level skill created inside the sandbox was discarded on exit, and the
agents/, commands/ and prompts/ directories the tools also read were not
writable at all (except Copilot's skills/ and agents/). Tools could
therefore not author their own extensions across sessions.
Replace build_personal_skills_mounts with two helpers in the library,
built on _mount_authoring_dir:
- build_authoring_dir_mounts KIND DIR...: create DIR on the host (not
under --dry-run) and bind it read-write as a whole at its logical
path, so anything created inside the sandbox persists.
- build_personal_authoring_mounts KIND DIR...: the same, but only for
directories that already exist (alternate search locations).
Skills symlinked in from a separate repository would dangle in a
whole-directory bind, and bwrap refuses to bind over a symlink inside a
bound directory. So the resolved target of each symlink entry is also
bound read-write at its own path; only that entry is exposed, not the
repository around it. Targets, or directories, that resolve into or
contain a blocked location are skipped with a warning. If the authoring
directory is itself reached through a symlink, fall back to the previous
per-entry binds (new top-level entries are then not persisted).
- bwopencode, bwopencode2: {skills,agents,commands} under the config
dir (now skipped by resolve_config_symlinks so the read-only bind
does not shadow them); ~/.claude/{skills,agents,commands} and
~/.agents/{skills,agents} when present
- bwclaude: {skills,agents,commands} under the Claude home
- bwcodex: ~/.codex/{skills,prompts}; ~/.agents/skills when present
- bwcopilot: ~/.copilot/{skills,agents} via the shared helper (replaces
the inline bind and mkdir); ~/.agents/skills when present
The deployment-managed read-only mounts are unchanged. Documented in
the launcher headers and a reworked "Authoring directories" section of
docs/bw-wrappers.md.
Based on the approach in NSLS2#4. Items from that PR
already on this branch (haiku model override, dropped gateway hostname
in bwclaude messages) are not repeated.
Assisted-by: OpenCode:claude-sonnet-5-5
_mount_ro_dir_if_needed bound a tool's on-PATH directory whole after
only _check_path_safe, which rejects a path that IS or sits inside a
blocked location but not a directory that CONTAINS one. An npm CLI
symlinked from a directory such as ~/.config (now mounted too, so the
invoked path exists in the sandbox) exposed that entire directory,
including an existing ~/.config/gh/hosts.yml, to the agent.
The root cause was that the two checks had to be paired by hand at every
bind site, and one site forgot. Make the safe thing the default:
- _check_bind_safe CANON CONTEXT: the check for any path about to be
bind-mounted. It runs _check_path_safe and then refuses a path that
contains a blocked location. Used by the working directory,
--ro-path/--rw-path, _mount_ro_dir_if_needed (so the npm prefix and
every on-PATH tool directory) and _bwrap_canonicalize_safe.
- _bwrap_bind_blocked CANON: the non-exiting form, for the authoring
directory mounts that skip a candidate with a warning. It replaces
their `! (_check_dir_contains_blocked ...) 2> /dev/null` subshell
construction.
- _bwrap_contains_blocked CANON: the shared predicate, non-exiting,
which reports the offending entry in _BWRAP_CONTAINED_BLOCKED.
- _check_dir_contains_blocked is removed so it cannot be called
without its companion again. _check_path_safe now says to use
_check_bind_safe when binding.
_check_path_safe stays for paths that are validated but not bound whole:
tool-home roots (_check_dir_safe) and, via the new "root" mode of
_bwrap_canonicalize_safe, CLAUDE_CONFIG_DIR and the opencode/opencode2
config directories. Those only have selected subpaths mounted, and the
opencode2 config directory is in EXTRA_BLOCKED_DIRS, so a contains-check
there would refuse every launch.
_bwrap_canonicalize_safe now applies the full check by default, which
newly refuses an opencode/opencode2 config entry that is a symlink to a
directory containing a blocked location (e.g. plugins -> ~/dotfiles
where ~/.ssh -> ~/dotfiles/ssh); such an entry is bound whole.
Addresses review comments on NSLS2#25 (lib/bwrap_sandbox_lib.sh,
_mount_ro_dir_if_needed; reported twice).
Assisted-by: OpenCode:claude-sonnet-5-5
build_extra_path_mounts opened with `[[ ${#EXTRA_RO_PATHS[@]} -eq 0 &&
${#EXTRA_RW_PATHS[@]} -eq 0 ]] && return 0`. Both arrays are empty in an
ordinary launch, and the project documents bash 4.x support, where
expanding empty arrays under `set -u` is fragile before bash 4.4.
Test for "neither array has elements" with the guarded expansion
`[[ -z "${A[*]+x}${B[*]+x}" ]]`, the same style the loops below already
use for their element expansions.
Addresses a review comment on NSLS2#25
(lib/bwrap_sandbox_lib.sh, build_extra_path_mounts).
Assisted-by: OpenCode:claude-sonnet-5-5
build_binary_masks ran right after the base sandbox, before every other mount. bwrap applies binds in order and a later bind that covers a path hides mounts beneath it, so an explicit `--ro-path /usr` (or /usr/bin, or any ancestor of a masked binary) re-bound the host tree over the /dev/null masks and made ssh, sudo, nc and the rest executable again. Confirmed with bwrap directly: mask-then-rebind leaves /usr/bin/true executable; rebind-then-mask leaves it a character device. Move build_binary_masks to the end of the mount sequence in all five launchers, after build_extra_path_mounts (and, in bwcopilot, after the settings and managed-skills binds that follow it). The masks only need the base /usr bind beneath them, so nothing else depends on the old position. Document the ordering requirement on build_binary_masks and in docs/bw-wrappers.md. Not changed: the /dev/tty mask in build_proc_dev_tmp has the same weakness (`--ro-path /dev` is accepted and rebinds /dev), as does the rest of the base mount set. That is a separate decision about which paths user mounts may cover, left for a follow-up. Addresses a review comment on NSLS2#25 (lib/bwrap_sandbox_lib.sh, build_extra_path_mounts). Assisted-by: OpenCode:claude-sonnet-5-5
docs/gpg-passwd.md still said the library requires bash >= 4.3 and is not zsh-safe because of a `local -n` nameref. That nameref was replaced by validated `eval` indirection (portable to zsh), so the warning now contradicts both the code and the library's own header, and wrongly tells zsh users not to source it. Replace it with a statement that bash and zsh are both supported and why the eval is safe, and bring the rest of the page in line with what the function enforces: required-variable names must be plain shell identifiers (not only free of the reserved __gpgpw_ prefix), and return code 2 also covers an invalid or reserved name. Checked by sourcing the library under bash and zsh with a stubbed gpg: a valid required variable returns 0, an empty one 2, an injection-shaped name 2 and a reserved name 2 in both shells. Addresses a review comment on NSLS2#25 (docs/gpg-passwd.md). Assisted-by: OpenCode:claude-sonnet-5-5
build_extra_path_mounts ordered the validated --ro-path/--rw-path set by
piping the keys through a newline-delimited `sort` into `mapfile -t`. A
path containing an embedded newline passes validation (it is a legal
file name) but was split into two entries by that pipeline, and the
following `_mode_of[...]` lookup on a half-path then failed under
`set -u`, aborting the launcher with "unbound variable":
bwopencode --ro-path "$(printf '/tmp/a\nb')" --dry-run
Pass the keys NUL-delimited end to end (`printf '%s\0'`, `sort -z`) and
collect them with a `read -r -d ''` loop. mapfile -d would be shorter
but needs bash 4.4, and the wrappers still support older 4.x. The
ancestors-first ordering is unchanged: an ancestor is still a byte
prefix of its descendants, so it sorts before them.
Checked with an embedded-newline directory: it is mounted once at its
real name, and a --rw-path nested under it plus a sibling --ro-path come
out in the expected ancestors-first order.
Addresses a review comment on NSLS2#25
(lib/bwrap_sandbox_lib.sh, build_extra_path_mounts).
Assisted-by: OpenCode:claude-sonnet-5-5
Three problems in _mount_authoring_dir, found while checking the review comments on NSLS2#25 against it: - "Already covered" was decided from _MOUNTED_PREFIXES, which records that an ancestor was bound but not that it is still visible. With the tool home inside the working directory (CLAUDE_CONFIG_DIR=$PWD/ .claude, CODEX_HOME=$PWD/.codex) the working-directory bind registers the repository, then the tool-home tmpfs hides the subdirectory; the authoring directory was treated as covered, never mounted, and its skills vanished from the sandbox. Decide it from the effective mount order instead: _path_visible_from_host walks BWRAP_ARGS and the path is covered only if the LAST mount over it is an identity bind (a later --tmpfs or a bind from another source hides it). This replaces _path_already_mounted and is used for the directory and for symlink targets. - The entry loops used a bare `*` glob, which skips dot-prefixed entries: a symlinked `.system` skill was left dangling (whole- directory mode) or missing (symlinked-directory fallback). Iterate `*`, `.[!.]*` and `..?*`. - A primary directory to be created was resolved with `readlink -f`, which prints nothing when a parent is missing, so on a fresh install (no ~/.config/opencode yet) the function returned silently and the directory was neither created nor mounted. Use `readlink -m`. Fresh homes now get working, persistent skills/agents/commands directories for bwopencode, bwclaude and bwcopilot. Checked: a skills directory under a tmpfs-hidden working directory is mounted and readable; dot-prefixed entries (real and symlinked) appear in both modes; fresh homes persist files written in the sandbox; a working directory containing the authoring directory is still not bound twice; --dry-run creates nothing. Addresses two review comments on NSLS2#25 (lib/bwrap_sandbox_lib.sh, build_personal_skills_mounts coverage check and dot-prefixed entries), which were written against the code this branch's authoring-directory work replaced. Assisted-by: OpenCode:claude-sonnet-5-5
Need at least an empty file to not be surprised to hit their providers. Assisted-by: opencode:claude-sonnet-5.5
…cations
bwopencode2 kept OpenCode 2 apart from OpenCode 1 by pointing all four
XDG_* variables at ~/.config|.local/share|.local/state|.cache/opencode2
inside the sandbox. That forced a workaround for every other tool that
reads $XDG_CONFIG_HOME or $XDG_CACHE_HOME (git, pixi, ccache), and made the
sandbox differ from every other wrapper.
Keep the opencode2 directories as the HOST convention (they match the
ansible `opencode2` wrapper and keep OpenCode 2 config, data and cache apart
from OpenCode 1 on the host), but mount them at the standard locations in
the sandbox:
host ~/.config/opencode2/opencode -> ~/.config/opencode
host ~/.local/share/opencode2/opencode -> ~/.local/share/opencode
host ~/.cache/opencode2/opencode -> ~/.cache/opencode
ephemeral ~/.local/state/opencode
(under $XDG_CONFIG_HOME etc. as the shared library resolves them, as
bwopencode does). No XDG_* variable is set any more; OpenCode 2 finds its
directories at the defaults.
- The variables are split into host sources (OC2_CONFIG_SRC, OC2_DATA_SRC,
OC2_CACHE_SRC) and sandbox destinations (OC2_CONFIG_DIR, OC2_DATA_DIR,
OC2_CACHE_DIR, OC2_STATE_DIR). Config entries are bound read-only at
OC2_CONFIG_DIR/<name>; the data and cache directories are bound from
their host sources; project-dotfiles stay under the host data directory.
The minimal-config check now looks at the host config directory
(OC2_CONFIG_SRC), since OC2_CONFIG_DIR is the sandbox path.
- service.json is still never mounted, and the host opencode2 config
directory stays in EXTRA_BLOCKED_DIRS.
- build_xdg_mirror_mounts is removed. It existed only to symlink git,
pixi and ccache config into the opencode2 XDG homes; with the standard
locations they find the library's mounts directly. The library flag
_BWRAP_XDG_GIT_HAS_FILES, which only it read, goes too.
- The read-write skills/agents/commands directories live under the host
config directory and are mounted under the sandbox one.
_mount_authoring_dir gains an optional SRC (the host directory to mount
at DIR) behind a new build_authoring_dir_mounts_from. Because host and
sandbox paths differ, a relative symlink inside such a directory would
resolve against the new location and dangle, so it is skipped with a
warning; absolute symlinks work as before.
OpenCode 1's config, data and cache are never mounted into bwopencode2:
OpenCode 2 migrates the OpenCode 1 session database one way and cannot load
its plugins, so a fallback to the plain ~/.config/opencode was considered
and dropped. Data and cache likewise always come from the opencode2 host
directories.
Documented in the launcher header, --help and docs/bw-wrappers.md (the
config/data/cache table, the isolation notes, and the removed git, pixi and
ccache mirror paragraph).
Assisted-by: OpenCode:claude-sonnet-5-5
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved mount-security issues and OpenCode launch regressions affect isolation and persistence.
1 open issue
6 resolved since last review
Review effort: Balanced
223b82c moved build_binary_masks after the user --ro-path/--rw-path mounts, but left it ahead of build_env_vars and the per-launcher env builder. Those can still emit binds: bwcodex's build_codex_env calls build_ssl_cert_file_mount, which --ro-binds the canonical SSL_CERT_FILE. A bind placed after a mask hides it, so SSL_CERT_FILE pointing at a masked binary (or a file whose canonical path is one) re-exposed that binary. Run build_binary_masks last in bwclaude, bwcodex, bwcopilot, bwopencode and bwopencode2, after the env builders, immediately before the dry-run / launch step. Env builders only add --setenv otherwise, so nothing else changes; the argument vector differs only in order. Update the ordering requirement on build_binary_masks and in docs/bw-wrappers.md accordingly. Checked with --dry-run on all five launchers (with --ro-path /usr): the last mount flag is the /dev/null mask in each. For bwcodex with SSL_CERT_FILE set, the cert bind used to come after the masks (line 250 vs masks ending at 223) and now comes before them (181 vs masks starting at 187). The masked-binary override itself could not be reproduced here because this session runs inside a sandbox in which the masked binaries are already /dev/null; ordering was confirmed with bwrap directly (rebind-then-mask leaves a character device). Addresses the remaining review comment on NSLS2#25 (lib/bwrap_sandbox_lib.sh, build_binary_masks). Assisted-by: OpenCode:claude-sonnet-5-5
Stuart B. Wilkins (stuwilkins)
approved these changes
Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Description
Lots of (🦜 aided) work to improve the bash scripts
Testing
Manual
Checklist
pixi run -e dev lintlocally (or wait for CI) — see.github/workflows/lint.ymlrunansibledeployment)