chore: cross-crate correctness and security fixes by imjustprism · Pull Request #196 · endevco/aube

imjustprism · 2026-04-21T18:00:34Z

security

workspace: now validates against local version, any workspace: prefix used to short-circuit so workspace:^2.0.0 linked a local 1.0.0 without complaint
allowlist bypass via npm: alias closed at every policy.decide site, h3-safe: npm:h3@0.19.0 no longer sneaks past an h3 entry
aube dlx forces ignore-scripts so transient installs do not inherit user's global allow-builds
aube run args shell-quoted per platform, no more $ / backtick / ; injection when forwarding args
publish ignore list covers .env / .env.* / .ssh / .aws / .gnupg / id_rsa / *.pem / *.key / *.p12
install ancestor walk stops at $HOME, empty-dir install cannot climb into user home and attach to a parent project
libc detection runs at runtime (probes /lib/ld-musl-* vs /lib*/ld-linux-*), not compile-time cfg
catalog entry pointing at another catalog rejected with a clear error
peer-context max-iter is error level not warn, convergence failures no longer silent in CI
aube audit --ignore-registry-errors exits 2 with degraded status instead of claiming clean

correctness

atomic writes for aube-lock.yaml (all 6 formats), .aube-state, .aube-applied-patches.json, package.json mutations in add/remove
CAS import fsyncs before close, shard-missing fallback switched from truncate-in-place to atomic create_new
detect_strategy probes cross-FS (store vs project) so store on a different mount no longer silently falls back to per-file copy
tarball extract rejects Windows reserved names / trailing dot-space / control chars on win32 only
parser tolerates leading UTF-8 BOM, null dep maps, string-form workspaces ("packages/*"), with targeted error for package.json JSONC comments
workspace glob skips node_modules/ at any depth so packages/** stops picking up installed deps
workspace member without a version is a hard error instead of silent 0.0.0 fallback
latest-tag-missing packument falls back to highest stable version, no more spurious NoMatch
0.0.0-0 workspace placeholder matches bare * range
graph_hash allow_build fed registry_name, not in-tree alias

resources / concurrency

fetch loops use JoinSet, first-error aborts outstanding siblings instead of detaching them
virtual store sweeps stale .tmp--* dirs on each install to reclaim prior-crash leftovers
pnpmfile afterAllResolved child has kill_on_drop
Windows remove_dir_all retries on sharing violations / access-denied with backoff

ux

signal-aware exit codes (128+signum) in aube run / dlx / exec, SIGKILL no longer collapses to plain 1
aube run lists available scripts from package.json
policy warnings and peer errors routed through a progress-safe writer so output does not smear with the install bar
branch-lockfile merge prints conflicts per line, integrity mismatches flagged explicitly as supply-chain signal
aube run sets npm_lifecycle_event / npm_package_name / npm_package_version / INIT_CWD, was a bare env before
aube run and aube exec take allow_hyphen_values so aube run build --flag works without the -- separator
root script failure error includes hook name, no more bare "exit status 1"

compat

publishConfig.registry / .tag / .access honored with CLI > manifest precedence
.npmrc backslash line continuation supported (npm ini semantics)
HOME falls back to USERPROFILE on Windows in expand_setting_path, user_npmrc_path, registry config home_dir
frozen fast-path state hash now covers .npmrc bytes, host (os+arch+libc), and patches dir + sidecar
find_workspace_root and find_workspace_yaml_root also cap the ancestor walk at $HOME

security - workspace:<range> now validates against local version, any workspace: prefix used to short-circuit so `workspace:^2.0.0` linked a local 1.0.0 without complaint - allowlist bypass via `npm:` alias closed at every policy.decide site, `h3-safe: npm:h3@0.19.0` no longer sneaks past an `h3` entry - aube dlx forces ignore-scripts so transient installs do not inherit user's global allow-builds - aube run args shell-quoted per platform, no more `$` / backtick / `;` injection when forwarding args - publish ignore list covers .env / .env.* / .ssh / .aws / .gnupg / id_rsa / *.pem / *.key / *.p12 - install ancestor walk stops at $HOME, empty-dir install cannot climb into user home and attach to a parent project - libc detection runs at runtime (probes /lib/ld-musl-* vs /lib*/ld-linux-*), not compile-time cfg - catalog entry pointing at another catalog rejected with a clear error - peer-context max-iter is error level not warn, convergence failures no longer silent in CI - aube audit --ignore-registry-errors exits 2 with degraded status instead of claiming clean correctness - atomic writes for aube-lock.yaml (all 6 formats), .aube-state, .aube-applied-patches.json, package.json mutations in add/remove - CAS import fsyncs before close, shard-missing fallback switched from truncate-in-place to atomic create_new - detect_strategy probes cross-FS (store vs project) so store on a different mount no longer silently falls back to per-file copy - tarball extract rejects Windows reserved names / trailing dot-space / control chars on win32 only - parser tolerates leading UTF-8 BOM, null dep maps, string-form workspaces ("packages/*"), with targeted error for package.json JSONC comments - workspace glob skips node_modules/ at any depth so `packages/**` stops picking up installed deps - workspace member without a version is a hard error instead of silent 0.0.0 fallback - latest-tag-missing packument falls back to highest stable version, no more spurious NoMatch - 0.0.0-0 workspace placeholder matches bare `*` range - graph_hash allow_build fed registry_name, not in-tree alias resources / concurrency - fetch loops use JoinSet, first-error aborts outstanding siblings instead of detaching them - virtual store sweeps stale .tmp-<pid>-* dirs on each install to reclaim prior-crash leftovers - pnpmfile afterAllResolved child has kill_on_drop - Windows remove_dir_all retries on sharing violations / access-denied with backoff ux - signal-aware exit codes (128+signum) in aube run / dlx / exec, SIGKILL no longer collapses to plain 1 - aube run <missing> lists available scripts from package.json - policy warnings and peer errors routed through a progress-safe writer so output does not smear with the install bar - branch-lockfile merge prints conflicts per line, integrity mismatches flagged explicitly as supply-chain signal - aube run sets npm_lifecycle_event / npm_package_name / npm_package_version / INIT_CWD, was a bare env before - aube run and aube exec take allow_hyphen_values so `aube run build --flag` works without the `--` separator - root script failure error includes hook name, no more bare "exit status 1" compat - publishConfig.registry / .tag / .access honored with CLI > manifest precedence - .npmrc backslash line continuation supported (npm ini semantics) - HOME falls back to USERPROFILE on Windows in expand_setting_path, user_npmrc_path, registry config home_dir - frozen fast-path state hash now covers .npmrc bytes, host (os+arch+libc), and patches dir + sidecar - find_workspace_root and find_workspace_yaml_root also cap the ancestor walk at $HOME

greptile-apps · 2026-04-21T18:16:03Z

Greptile Summary

This PR is a broad cross-crate hardening pass covering security fixes (allowlist bypass via npm: aliases, workspace:<range> validation, shell injection, secret-file publish blocklist, ancestor-walk HOME boundary), correctness improvements (atomic writes for all 6 lockfile formats, state file, and package.json mutations, JoinSet fetch loops, cross-FS strategy probe, runtime libc detection), and UX improvements (signal-aware exit codes, available-scripts listing, progress-safe output, publishConfig support).

P1 — prune_sidecar_entries scoped-package gap: rsplit_once('@') on a bare scoped name like \"@types/node\" splits at position 0, yielding an empty left-hand side. The resulting base = \"\" never matches the package name, so the entry is silently retained in trustedDependencies / neverBuiltDependencies / onlyBuiltDependencies arrays. The security invariant (removing a package also removes its build-approval sidecar) is broken for scoped packages without an explicit version suffix.

Confidence Score: 4/5

Safe to merge after fixing the scoped-package pruning bug in prune_sidecar_entries; all other changes are well-reasoned and clearly explained.

One P1 finding: rsplit_once('@') on a bare scoped package name yields an empty left-hand side, silently skipping the sidecar prune for the exact case the security comment is warning about. Everything else — atomic writes, JoinSet fetch loops, shell quoting, INIT_CWD, signal-aware exit codes, runtime libc detection, allowlist alias bypass fixes — is correct and well-tested.

crates/aube/src/commands/remove.rs — prune_sidecar_entries array matching for scoped packages

Security Review

prune_sidecar_entries scoped-package bypass (crates/aube/src/commands/remove.rs): bare scoped names like @scope/pkg are not removed from trustedDependencies/neverBuiltDependencies/onlyBuiltDependencies arrays due to the rsplit_once('@') empty-left-side edge case. A future reinstall of the removed package can inherit stale build-approval entries.
All other security fixes (allowlist alias bypass via registry_name(), workspace:<range> version validation, shell-injection quoting, --ignore-scripts forced on dlx, publish secret-file blocklist, HOME ancestor-walk boundary) appear correctly implemented.

Important Files Changed

Filename	Overview
crates/aube/src/commands/remove.rs	Adds atomic manifest write and `prune_sidecar_entries` on remove; the `rsplit_once('@')` logic fails to match bare scoped package names in array-shaped fields, leaving stale build-approval entries.
crates/aube-linker/Cargo.toml	Adds `serde_json` and `tempfile` deps; `tempfile` uses `"3"` instead of `{ workspace = true }`, creating an inconsistency with other workspace crates.
crates/aube-linker/src/lib.rs	Adds `sweep_stale_tmp_dirs`, `remove_dir_all_with_retry` (Windows backoff), cross-FS strategy probe, and atomic `write_applied_patches`; logic looks correct.
crates/aube-lockfile/src/lib.rs	Adds `atomic_write_lockfile` helper used by all 6 lockfile format writers; temp-file + fsync + rename pattern is correct.
crates/aube-resolver/src/lib.rs	Fixes `workspace:<range>` validation against local version, adds catalog-chain rejection, and adds `highest_stable_version` fallback for missing `latest` dist-tag; logic is correct.
crates/aube/src/commands/run.rs	Extracts shared `build_script_command` helper, adds npm-compat env vars, fixes INIT_CWD to use actual invocation dir via `dirs::cwd()`, and applies `exit_code_from_status`.
crates/aube-scripts/src/lib.rs	Adds `shell_quote_arg` (Unix single-quote / Windows double-quote with `%%` for `%VAR%`) and `exit_code_from_status` (128+signum on Unix); both are correct.
crates/aube/src/commands/install/mod.rs	Switches fetch loops to `JoinSet` for abort-on-first-error, fixes `policy.decide` callers to use `registry_name()`, uses cross-FS strategy probe, and surfaces merge conflicts per-line via `safe_eprintln`.
crates/aube/src/state.rs	Switches `write_state` to atomic tempfile+persist and extends the frozen fast-path hash to cover `.npmrc` bytes, host triple, and patches dir.
crates/aube/src/dirs.rs	Caps ancestor walks in `find_project_root`, `find_workspace_root`, and `find_workspace_yaml_root` at `$HOME`/`USERPROFILE`; logic is correct.

Comments Outside Diff (1)

crates/aube/src/commands/remove.rs, line 302-312 (link)

Scoped packages silently skipped in array-shaped sidecar pruning

rsplit_once('@') on a bare scoped name like "@types/node" splits at the only @ (position 0), yielding Some(("", "types/node")). The resulting base = "" never equals "@types/node", so the entry is retained instead of pruned.

The security invariant the code is trying to enforce — removing a package also removes its build-approval sidecar entries — is broken for any scoped package that appears without an explicit version suffix (the common form in trustedDependencies/neverBuiltDependencies).

Fix: detect the empty-left-side case and fall back to the whole string as the base name:

_{Reviews (2): Last reviewed commit: "run: INIT_CWD uses invocation cwd, dedup..." | Re-trigger Greptile}

jdx · 2026-04-22T02:37:52Z

@greptileai is the P1 issue resolved?

greptile-apps · 2026-04-22T02:38:19Z

Yes — the P1 thread (JSON injection via unescaped error message in audit.rs:191) is marked as resolved. The fix uses serde_json::json! to safely serialize the error string rather than interpolating it directly into a raw JSON literal.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules and .greptile/config.

imjustprism added 3 commits April 21, 2026 19:58

linker: drop needless return in remove_dir_all_with_retry

48bba62

test: audit --ignore-registry-errors now exits 2 degraded

275c155

greptile-apps Bot reviewed Apr 21, 2026

View reviewed changes

Comment thread crates/aube/src/commands/audit.rs

Comment thread crates/aube/src/commands/run.rs Outdated

Comment thread crates/aube-scripts/src/lib.rs

imjustprism added 3 commits April 21, 2026 20:23

audit: serialize degraded json via serde_json to escape quotes

cc05516

scripts: escape %VAR% in windows shell_quote_arg

c194203

run: INIT_CWD uses invocation cwd, dedupe script command builder

8e96436

jdx merged commit ea87e83 into endevco:main Apr 22, 2026
15 checks passed

imjustprism deleted the general-fixes branch April 25, 2026 10:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: cross-crate correctness and security fixes#196

chore: cross-crate correctness and security fixes#196
jdx merged 6 commits intoendevco:mainfrom
imjustprism:general-fixes

imjustprism commented Apr 21, 2026

Uh oh!

greptile-apps Bot commented Apr 21, 2026 •

edited

Loading

Security Review

Important Files Changed

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdx commented Apr 22, 2026

Uh oh!

greptile-apps Bot commented Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

imjustprism commented Apr 21, 2026

Uh oh!

greptile-apps Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdx commented Apr 22, 2026

Uh oh!

greptile-apps Bot commented Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Apr 21, 2026 •

edited

Loading