Harden safety, executor & CI; dedup shell quoting#1
Open
meganfoster-dotcom wants to merge 2 commits into
Open
Conversation
Five targeted hardening fixes from the code review, no behavioral rewrites: - ci: add GitHub Actions workflow (typecheck, lint, build, test on Node 18/20/22). The suite is fully offline, so no API keys/credits are used. - executor: wire a per-step wall-clock timeout into the setup path so a hung command (stuck install, stdin prompt, unreachable registry) can no longer block forever. Generous per-type defaults, overridable via SETUPR_STEP_TIMEOUT_MS; timeouts surface a clear terminated message. - safety: remove dead-code ternary (force ? confirm : confirm) — high risk now plainly always confirms and --force cannot bypass it. Normalize forceCanSkipConfirmation to `risk === "medium"` in both safety.ts and engine.ts so the two agree. - safety: tighten secret detection to match inline KEY=value credential assignments and recognizable secret value literals instead of bare words, removing false-positive confirmations (e.g. `npm i next-auth`). - refactor: extract the duplicated shellQuote helper into src/util/shell.ts and import it in security, verification, and product. - docs: document the safety threat model in SECURITY.md (best-effort denylist + best-effort redaction, not a sandbox). Adds 9 regression tests (186 total). typecheck, lint, build, and the full suite pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The vitest 4 / rolldown test toolchain imports `styleText` from node:util, which only exists on Node 20+, so the test step fails on Node 18. The published CLI bundle still targets Node 18; CI builds and tests on the versions the dev toolchain supports. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Five targeted hardening fixes from the recent code review. No feature work or behavioral rewrites — each change is small and tested.
What's in here
.github/workflows/ci.yml) — runs typecheck, lint, build, and the full test suite on Node 18/20/22 for every push and PR. The suite is fully offline, so no AI provider keys or credits are used in CI.executeStepnow passes a wall-clock timeout torunCommand, so a hung command (stuck install, a command waiting on stdin, an unreachable registry) can no longer block the run forever. Generous per-type defaults (10 min for installs/builds, 2 min otherwise), overridable viaSETUPR_STEP_TIMEOUT_MS. Timeouts surface a clear "timed out … and was terminated" message.src/agent/safety.ts) — removed theforce ? "confirm" : "confirm"ternary; high risk now plainly always confirms and--forcecannot bypass it.forceCanSkipConfirmationis normalized torisk === "medium"in bothsafety.tsandengine.tsso the two agree.auth/token/secret, raising false-positive confirmations on benign commands (e.g.npm i next-auth). It now matches inlineKEY=valuecredential assignments and recognizable secret value literals (sk-…,ghp_…,AIza…, PEM private keys, etc.).shellQuote— extracted the helper that was copy-pasted in three files intosrc/util/shell.ts.Plus a
SECURITY.mdthreat-model section documenting that the safety layer is a best-effort, defense-in-depth denylist (not a sandbox) and redaction is best-effort.Verification
npm run typecheck✅npm run lint✅npm run build✅npm test✅ — 186 passing (added 9 regression tests covering the secret-pattern fix, force rules,shellQuote, and the timeout helper)🤖 Generated with Claude Code