Skip to content

fix: merge latest dev updates into main#22

Merged
nadav-node9 merged 35 commits intomainfrom
dev
Mar 20, 2026
Merged

fix: merge latest dev updates into main#22
nadav-node9 merged 35 commits intomainfrom
dev

Conversation

@nadav-node9
Copy link
Contributor

Auto-generated PR

Merge latest dev changes into main to trigger a release.

⚠️ Important: When you click Squash and Merge, ensure the commit message starts with:

  • fix: to publish a Patch release (0.0.X)
  • feat: to publish a Minor release (0.X.0)
    If it starts with chore:, no NPM package will be published!

nadavis and others added 30 commits March 14, 2026 16:41
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- native.ts: add extractContext + formatArgs with matchedField/matchedWord
  tracing for "Context Sniper" popup — shows dangerous word in context
- core.ts: extend evaluatePolicy return with matchedField/matchedWord;
  per-field scan after dangerous word found; pass through authorizeHeadless
- daemon/index.ts: gate SSE broadcast and browser open on browser config flag
- LICENSE/package.json/README.md: MIT → Apache-2.0
- .github/workflows/ai-review.yml: add paths-ignore to prevent self-modification
- scripts/ai-review.mjs: upgrade to claude-sonnet-4-6, max_tokens 2048

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Wrap diff in <diff>...</diff> markers with untrusted-content notice to mitigate prompt injection
- Surface truncation note in posted PR comment when diff exceeds MAX_DIFF_CHARS
- Downgrade API errors to warning comments + exit 0 so Anthropic outages don't block PRs
- Pin @anthropic-ai/sdk@0.78.0 and @octokit/rest@22.0.1 to prevent supply-chain drift
- Add explicit permissions block (contents: read, pull-requests: write)
- Exclude dependabot[bot] from triggering review
- Add fetch-depth: 0 to checkout step

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… main

- ai-review.yml: replace AUTO_PR_TOKEN with GITHUB_TOKEN (permissions block
  already scopes it correctly — no broad PAT needed)
- ai-review.yml: add --ignore-scripts to npm install to block malicious
  postinstall hooks from transitive dependencies
- sync-dev.yml: new workflow — after every push to main, merge main back into
  dev so release-bot version bumps don't cause recurring README conflicts on
  the next dev -> main PR

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move @anthropic-ai/sdk and @octokit/rest into devDependencies and switch the
ai-review workflow from bare npm install to npm ci --ignore-scripts. This
locks all transitive dependencies to the committed lockfile, eliminating
supply-chain drift on every CI run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- context-sniper.ts (new): shared RiskMetadata type, smartTruncate,
  extractContext (returns {snippet, lineIndex}), computeRiskMetadata
- native.ts: import from context-sniper, use .snippet on extractContext calls
- core.ts: add tier to evaluatePolicy returns; compute riskMetadata once in
  authorizeHeadless; pass it to initNode9SaaS, askDaemon, notifyDaemonViewer
- daemon/index.ts: store and broadcast riskMetadata in PendingEntry
- daemon/ui.html: renderPayload() uses riskMetadata for intent badge, tier,
  file path, annotated snippet, matched-word highlight; falls back to raw args

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the Node9 cloud responds with shadowMode:true (org is in shadow
mode), print a yellow warning to stderr instead of blocking the agent.
The developer sees exactly why it was flagged without being interrupted.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…er, and integration tests

- Add Zod v3 schema validation (config-schema.ts) with clear error messages for
  bad config.json — catches literal newlines, invalid regex, unknown keys, bad enums
- Fix silent JSON parse fallback in tryLoadConfig: bad config now warns to stderr
  instead of silently using DEFAULT_CONFIG (which had cloud:true causing unexpected
  browser/cloud popups when config was invalid)
- Fix auditLocalAllow fire-and-forget killed by process.exit: audit mode path now
  awaits the POST so SaaS receives the event before the process exits
- Gate all auditLocalAllow calls on approvers.cloud so cloud:false (privacy mode)
  never sends data to SaaS
- Fix double browser windows when cloud+browser both enabled: RACER 3 (browser)
  now skips when cloudEnforced, preventing duplicate daemon /check entries
- Fix calledFromDaemon guard on terminal status messages to prevent duplicate output
- Add check.integration.test.ts: 20 end-to-end tests spawning real node9 check
  subprocess with isolated HOME dirs and in-process mock SaaS server

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…plied

Previously tryLoadConfig warned about invalid fields (e.g. mode:"bad-mode")
but still returned the raw object, letting them override valid values from
higher-priority config layers. A project-level node9.config.json with
mode:"bad-mode" would override the global mode:"audit", bypassing audit mode
and triggering the full cloud approval race unexpectedly.

sanitizeConfig() now drops top-level keys that fail Zod validation so invalid
project configs cannot corrupt the effective merged config.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l test, improve comments

- Remove unreliable 200ms sleep from audit+cloud test: auditLocalAllow is
  awaited before process.exit so the POST is done by the time the subprocess
  closes; if it ever races here it would be a production bug too
- Add task* wildcard test: task_drop_all_tables must be fast-pathed to allow
  (documents the intentional security trade-off of user-configured wildcards)
- Expand runCheck docstring explaining why cwd=tmpHome is needed alongside
  HOME=tmpHome (avoids inheriting the repo's own node9.config.json)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…noredTools precedence tests

- Add `let resolved = false` guard in runCheckAsync to prevent double-resolve
  when child.kill() is called on timeout (close event fires after kill)
- Fix mockServer.close() in afterEach to return a Promise (was fire-and-forget)
- Document NODE9_TESTING=1 behavior in file header comment
- Add runCheck/runCheckAsync raw string support for malformed payload testing
- Add section 10: malformed JSON payload tests (non-JSON, empty, partial JSON)
- Add ignoredTools precedence test: task* wildcard + dangerous word in input
  documents that ignoredTools fast-path bypasses dangerousWords (by design)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CLI intentionally exits 0 on unparseable JSON (fail-open policy):
a transient serialization error must not block the AI session mid-flight.
The test was asserting the opposite. Updated all three malformed-payload
tests to verify graceful failure (no crash/stack trace) rather than an
error exit code.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
core.ts:
- fix review-git-push regex: literal space → \s+ so "git  push" can't bypass
- fix getConfig(): environments block was always hardcoded {} and never merged
  from global/project config files; now applyLayer() accumulates environments
  correctly so strict-mode env overrides actually work

examples/node9.config.json.example:
- remove dangerousWords that caused false positives; keep only mkfs + shred
  (catastrophic, unambiguous — everything else handled by smartRules)
- add enterplanmode/enterworktree/exitworktree to ignoredTools
- add execute_query, query, mcp__postgres__*, mcp__github__* to toolInspection
- fix allow-readonly-bash regex: "npm run(build|test)" → "npm run (build|test)"
  (was matching "runbuild"/"runtest" instead of "run build"/"run test")
- remove smartRules already covered by built-in defaults:
  review-delete-without-where, block-force-push, block-drop-database, review-sudo
- remove "push"/"git" rules entries (match tool *names*, never fire for bash)
- remove non-functional environments block (was silently ignored until above fix)
- add approvalTimeoutMs:30000, version:"1.0", expanded snapshot.tools + ignorePaths

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ule bypass, tests

core.ts:
- move mergedEnvironments declaration before applyLayer closure so the captured
  variable is clearly in scope (was hoisted at runtime but misleading to read)
- replace unsafe Partial<EnvironmentConfig> spread with field-level validation:
  only copies requireApproval when it is a boolean, ignoring any other input
- add version guard in tryLoadConfig: warns to stderr when a config file declares
  a version other than "1.0" so future schema changes can be caught early

examples/node9.config.json.example:
- add notMatches (&&|\|\||;\s*\S) condition to allow-readonly-bash rule so
  chained commands like "cat /etc/passwd && rm -rf /" are NOT fast-allowed
- tighten review-secrets-write file_path regex: add (^|[/\\]) path separator
  anchor so "notmy.env" no longer matches — only actual dotenv files do

tests (advanced_policy.test.ts):
- 6 new tests for allow-readonly-bash: verifies safe commands are allowed,
  &&/||/; chaining is rejected, pipe-only chains remain allowed
- 4 new tests for environments merge: project overrides global, type-unsafe
  values are dropped, multiple envs merge independently

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ets detection

- allow-readonly-bash: add notMatches for $() and backtick to prevent
  command substitution bypass
- allow-install-devtools: add notMatches guard for -g/--global flags
- review-secrets-write: change to conditionMode any and add path/filename
  field checks alongside file_path
- Add review-command-substitution rule (catches $() and backtick)
- Add review-global-install rule (catches npm/yarn/pnpm -g/--global)
- Add tests: $() bypass, backtick bypass, npm install -g, --global,
  review-secrets-write multi-field (path, filename), version mismatch
  warning/rejection paths

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n guard, secrets rule rename

- Restore rm and drop to dangerousWords (security regression from previous cleanup)
- Fix allow-readonly-bash notMatches: change ;\s*\S to bare ; so a
  trailing semicolon with no following content cannot bypass the guard
- Rename review-secrets-write → flag-secrets-access to reflect that it
  covers reads as well as writes; update reason string to match
- Add .env.bak test: dotfile backup IS flagged, notmy.env.bak is NOT
- Update all test fixtures to use ; instead of ;\s*\S to stay consistent
  with the corrected example config

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nadav-node9 and others added 3 commits March 17, 2026 07:30
- node9 undo now filters snapshots by cwd, so you only see
  snapshots from the current project
- add --all flag to show global snapshot history across all projects
- add helpful hint message when no local snapshots found but global ones exist
- add brew install to README quick start as recommended install method
- add homebrew tap notification step to release workflow

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

🤖 Claude Code Review

Review

.github/workflows/release.yml — Homebrew tap notification

Security concern: The VERSION is extracted via node -p "require('./package.json').version" and interpolated directly into a JSON string with shell variable expansion ("${VERSION}"). If package.json ever contains a malformed or adversarially crafted version string, this could break the JSON payload or, in a worst case, enable injection. Use jq or pipe through a JSON encoder rather than bare shell interpolation. A simpler fix: use jq -n --arg v "$VERSION" '{event_type:"new-release",client_payload:{version:$v}}'.

Minor: The AUTO_PR_TOKEN is reused for both semantic-release and the homebrew dispatch. If the homebrew tap lives in a different org or requires different scopes, this could silently fail (the -s curl flag suppresses output). Consider adding --fail to the curl call so a bad response fails the step visibly rather than silently.


README.md

Bug: The image src has been changed to "an" — this is clearly a broken/accidental edit and will render a broken image in the README. Must be reverted.


src/cli.tsundo --all flag

Correctness: getSnapshotHistory() is called once and stored as allHistory, with history filtered from it. This is correct and avoids a double call — good.

Edge case: parseInt(options.steps, 10) || 1 will silently treat --steps 0 as 1, which is reasonable, but --steps -5 gets clamped by Math.max(1, ...). That's fine, but worth a comment so future maintainers don't wonder why negative values are accepted without error.

Type safety: options.all is typed as boolean | undefined, which is correct for an optional flag. The options.all ? ternary handles undefined as falsy — no issue.

UX/security: The hint message leaks the count of other projects' snapshot directories implicitly (by checking allHistory.length > 0). In a multi-user or shared environment this is low risk, but worth noting since this is a security tool — snapshot metadata could reveal project paths.

No concerns with the logic path itself; the filtering is straightforward and the messaging improvement is genuinely useful.


Summary

Two issues require action before merge:

  1. Broken image src ("an") in README — likely an accidental edit, must be fixed.
  2. Shell injection risk in the curl JSON payload — use jq for safe serialization.

The undo --all CLI change is clean. No functional issues found in the core security path (no changes to core.ts, daemon/index.ts, or native.ts).


Automated review by Claude Sonnet

- restore broken README image src to original GitHub asset URL
- use jq for safe JSON serialization in homebrew dispatch curl call
- add --fail to curl so failed dispatch is visible in CI logs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

🤖 Claude Code Review

Review

CI/CD — Homebrew notification step

Security concern (medium): AUTO_PR_TOKEN is being reused for both semantic-release (which likely needs write access to this repo) and dispatching to the Homebrew tap repo. If the tap dispatch only needs repo:dispatch scope on node9-ai/homebrew-node9, this is over-privileged. Consider a separate narrowly-scoped token for the tap notification.

Correctness: The VERSION extraction runs after npx semantic-release, but semantic-release may bump package.json on disk during the run — this should be fine. However, if semantic-release fails or produces no release (e.g. no releasable commits), this step still fires and will dispatch with whatever the current version is. The step should be conditional on a release actually occurring:

if: steps.release.outputs.new_release_published == 'true'

Without this, spurious dispatches to the Homebrew tap will occur on every CI run against the release branch.

Minor: --fail on curl is good practice. No issues there.


CLI — undo --all flag

Correctness: The cwd comparison uses s.cwd === process.cwd() — this is a strict string equality check. On macOS, symlinks or path normalization differences (e.g. /private/tmp vs /tmp) could cause snapshots to be silently excluded even without --all. Consider normalizing both paths with path.resolve() before comparing.

Edge case: parseInt(options.steps, 10) || 1 — if a user passes --steps 0, parseInt returns 0, falsy, so it becomes 1. Then Math.max(1, 1) gives 1. The Math.max guard makes the || 1 redundant but they're at least consistent. Low risk, but worth a comment.

Type safety: options.all typed as boolean | undefined is correct for Commander's optional flag. No issues.

UX correctness: The improved empty-state message is a genuine improvement — no issues.


README

No functional issues. The Homebrew install path is now the recommended one, which is fine once the tap is published and stable.


Summary

Two actionable items:

  1. Guard the Homebrew dispatch step on new_release_published == 'true'
  2. Use a separate, scoped token for the tap dispatch, or at minimum document why the shared token is acceptable
  3. Normalize cwd paths in the undo filter to avoid symlink-related mismatches

The core proxy logic (core.ts, daemon/index.ts, native.ts) is untouched — no concerns there.


Automated review by Claude Sonnet

@github-actions
Copy link
Contributor

🤖 Claude Code Review

Review

CI/Workflow (release.yml)

Security concern: AUTO_PR_TOKEN is reused for both semantic-release (needs repo write access) and the Homebrew dispatch call. If the Homebrew tap repo is compromised or the dispatch endpoint is abused, the token's broader permissions are exposed. Consider a scoped, separate token (HOMEBREW_DISPATCH_TOKEN) with only repo dispatch rights on the tap repo.

Minor: The curl step has no timeout flag (--max-time). A hung GitHub API call will block the release job indefinitely. Add --max-time 30.

Version extraction: node -p "require('./package.json').version" runs after npx semantic-release, which is correct — semantic-release has already bumped package.json by this point. No issue here.


src/cli.ts (undo --all)

Correctness — cwd field assumption: The filter s.cwd === process.cwd() assumes every snapshot object has a cwd property. If getSnapshotHistory() returns snapshots created before this field was added (e.g., older snapshot files on disk), s.cwd will be undefined, and those snapshots will be silently excluded from the default (non---all) view without any indication. This could confuse users who have valid snapshots but see the "no snapshots found" message. At minimum, document the migration behavior or fall back to including snapshots with no cwd.

Type safety: The s.cwd field should be reflected in the snapshot type definition. If cwd is not in the type, this compiles only because of implicit any or an overly loose type — worth checking that noImplicitAny / strict are enforced and the type is updated.

Edge case: process.cwd() can vary based on how the CLI is invoked (symlinks, shell cd behavior). path.resolve(process.cwd()) would be safer for comparison.


README (README.md)

No issues. The Homebrew install instructions and related-projects section are straightforward documentation changes.


What's missing / test gaps

  • No test coverage for the --all flag path or the new "snapshots exist in other dirs" hint message.
  • No test for the s.cwd === undefined (legacy snapshot) case.

Summary

The CI token scoping is the most important issue to address before merging. The cwd undefined fallback in the undo command is a real correctness bug affecting users upgrading from older versions. Everything else is minor.


Automated review by Claude Sonnet

@nadav-node9 nadav-node9 merged commit 3e235d7 into main Mar 20, 2026
8 checks passed
nadav-node9 pushed a commit that referenced this pull request Mar 20, 2026
## [1.0.12](v1.0.11...v1.0.12) (2026-03-20)

### Bug Fixes

* merge latest dev updates into main ([#22](#22)) ([3e235d7](3e235d7))
@nadav-node9
Copy link
Contributor Author

🎉 This PR is included in version 1.0.12 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants