- Replaced time-based sleeps and polling loops with event-driven promise resolvers and fake timers across agent and tool tests.
- Migrated test suites to share in-memory auth storage and fixtures using lifecycle hooks.
- Updated catalog model definitions, metadata, and configurations.
- The earlier EBADF 'hardening' was chasing a Bun 1.4-canary quirk that
only triggers when bun test runs workspace-package files from the repo
root; the canonical runner (scripts/ci-test-ts.ts) always uses the
package cwd, where the original PR-authored pipe-based assertions pass.
- Restores the stronger stdout contracts from PRs #7135 and #7141.
- Bun 1.4 canary's posix_spawn intermittently rejects pipe-backed child
stdio (EBADF) inside test workers, failing telemetry-export and
shell-snapshot probes before their assertions ran.
- Probes now spawn with ignored stdio and capture output via exit status
or temp-file redirection; all behavioral assertions retained.
- The snapshot dir was one fixed name under the shared `os.tmpdir()`,
created 0700 because the script inlines env-var values referenced by
captured functions (#3470). The first account to run omp owns it, so on
a box where a second Unix account runs omp every bash tool call died
with `EACCES: permission denied, open
'/tmp/omp-shell-snapshots/snapshot-bash-<uuid>.sh'`.
- `recursive: true` swallows EEXIST and `mode` is ignored for an existing
dir, and the defensive `chmodSync` fails EPERM on a foreign-owned dir
and was already swallowed, so the code walked straight into a dir it
could not write.
- `mkdirSync` and the pre-create `writeFileSync` both sat outside any
try/catch, so the error escaped `getOrCreateSnapshot` into
`executeBash`. Nothing is cached on failure, so it repeated for every
command instead of degrading once.
- Scope the dir per uid (`omp-shell-snapshots-<uid>`) and guard both
filesystem calls: an unusable dir now logs at debug and returns `null`,
which callers already handle as "run without a snapshot".
getOrCreateSnapshot in shell-snapshot.ts pre-creates snapshotPath as an empty temp file and returns null on spawn failure, timeout, or nonzero exit without deleting it, leaving stale files in os.tmpdir()/omp-shell-snapshots/. Track whether snapshot creation succeeded and remove the pre-created file in a finally block on every failure path using fs.rmSync(snapshotPath, { force: true }), with best-effort error suppression so cleanup failures never propagate. Verified with `bunx tsc --noEmit -p packages/coding-agent/tsconfig.json` and `bun test packages/coding-agent/test/shell-snapshot.test.ts` (22 pass).
Closes#4236
The two getOrCreateSnapshot e2e tests hard-coded /usr/bin/bash and
/usr/bin/echo, both absent on macOS (bash is /bin/bash, echo is
/bin/echo). The symlink then pointed at a nonexistent target so
getOrCreateSnapshot returned null, and the replay invoked a missing
echo. Since #3470 is a macOS bug, the regression tests could not run on
the affected platform. Resolve bash via Bun.env.SHELL (mirroring
bash-executor.test.ts) with a /bin/bash fallback and an existsSync skip
guard, and resolve echo via Bun.which with a /bin/echo fallback.
PR #3474 second review: previous revision ran `umask 077` only BEFORE
sourcing the rc, so a typical `.bashrc`/`.zshrc` that calls
`umask 022` reopened the world-read window between the spawned shell's
first `>|` and the JS post-spawn chmod. Snapshot file (with inlined
env-var values) lived at 0644 for the full body of the script.
Two-layer fix:
- JS caller now pre-creates the snapshot file at 0600 with
`fs.writeFileSync(path, "", { mode: 0o600 })` before spawning. The
shell's `>|` (truncate) and `>>` (append) preserve the existing
inode mode, so the file is 0600 from byte zero regardless of the
spawned shell's umask state.
- Script also re-applies `umask 077` after the rc source so any
other file the script might create (none today, defensive) stays
private even when the rc resets umask.
New e2e regression test seeds a `.bashrc` containing `umask 022` and
asserts the resulting snapshot mode `& 0o077 === 0`.
PR #3474 review: the new export pass writes referenced env-var values
into a snapshot file under `os.tmpdir()/omp-shell-snapshots`. On Linux
where `os.tmpdir()` is `/tmp` and the umask is the default 022, the
file ended up world-readable (0644) until postmortem cleanup. A user
rcfile defining `deploy(){ curl -H "Authorization: $GITHUB_TOKEN" ...; }`
would have its token written verbatim to that file.
Three-layer mitigation:
- `umask 077` at the top of the snapshot script so the file is 0600
from the first byte (the shell creates it via redirection, not JS).
- JS caller now passes `mode: 0o700` to `mkdirSync` and chmods the
dir + file defensively after the script exits, covering pre-existing
dirs and exotic shells where the umask call might not take.
- Helper denylist gained the common secret-shaped name patterns
(`*TOKEN*`, `*SECRET*`, `*API_KEY*`, `*PASSWORD*`, `*PASSWD*`,
`*PRIVATE_KEY*`, `*ACCESS_KEY*`, `*CREDENTIAL*`, `*SESSION_KEY*`)
so even when the file is locked down, we don't materialise tokens
onto disk in the first place.
Tests cover both: a new helper-level test asserts none of the secret
names (or their values) appear in the export stream, and the e2e test
now stats the snapshot file + dir and asserts `mode & 0o077 === 0`.
generateSnapshotScript captured the user's shell functions via declare -f /
typeset -f and dropped everything except PATH on the export floor. mise
activate installs a mise() function whose body expands $__MISE_EXE; the
replay shell then ran `command "" "$@"` and died with
`command: command not found:` (exit 127). The same shape breaks asdf
shims, direnv-style helpers, and any other activation idiom that pairs a
shell function with a sidecar env var.
The snapshot script now scans captured function bodies for $VAR /
${VAR…} references and re-emits `export NAME='value'` for each name
that is currently set and not on a shell-internal denylist (PATH, HOME,
BASH_*, LC_*, …). getShellConfigFile also honours env.HOME so callers
(and tests) can target a sandboxed home — os.homedir() is cached by Bun
and ignores later process.env.HOME mutations.
Fixes#3470
brush-core's alias expander resolves aliases via
`value.split_ascii_whitespace()` (`crates/brush-core-vendored/src/interp.rs:1500`,
upstream brush issue reubeno/brush#57): each whitespace piece is dropped
into argv as-is, completely bypassing the shell parser. Any alias body
containing `(`, `)`, `|`, `&`, `;`, `<`, `>`, or `\`` therefore
turns the first piece into the command name, so Fedora's default
`alias which='(alias; declare -f) | /usr/bin/which …'` produces
`error: command not found: (alias;` for every `which` invocation.
The user's shell snapshot is generated by sourcing their real rc-file
under `/bin/bash` or `/bin/zsh` (so we can capture functions, options,
PATH) and then sourced by brush per-session. `sanitizeSnapshotForBrush`
now scans the emitted `alias -- NAME='VALUE'` lines after generation,
drops any whose decoded body contains those metacharacters, and rewrites
the file in place before caching. Compatible aliases (`ll='ls -l'`,
`gc='git --color=auto commit'`, embedded-quote `say='echo '\\''hi'\\'''`)
are preserved untouched; dropped names are logged at debug. brush then
falls through to whatever lives on `PATH`, which is what the user
expected when they ran `which` in the first place.
Covered by unit tests for the sanitizer (Fedora-which case, every
incompatible-metachar shape, every preserve case) and an integration
test that loads a poisoned snapshot and verifies `which sh` now exits
`0` with a real path.
Fixes#3234
can1357