chore(hooks+claude+oxfmt): hooks-mts conversion + sweeper + CLAUDE.md restructure + oxfmt JSDoc + logger-guard + auth-rotation-reminder#1286
Open
John-David Dalton (jdalton) wants to merge 12 commits intomainfrom
Conversation
Self-landable split from #1279. Combines the hook overhaul into one atomic PR. Path-guard infra - .claude/hooks/path-guard/ (hook + tests + segments.mts) - .claude/skills/path-guard/ (audit-and-fix skill) - .claude/skills/_shared/path-guard-rule.md (canonical rule) - scripts/check-paths.mts (the gate) - .github/paths-allowlist.yml (empty starter, full schema docs) - .claude/settings.json (wires hook on Edit|Write) - scripts/check.mts (invokes the gate) Token-guard hook - .claude/hooks/token-guard/ (renamed from token-hygiene; word- boundary match for sensitive env names; ALWAYS_DANGEROUS check skips when redaction pipeline is present) .sh → .mts hook conversion (Node 25+) - .git-hooks/_helpers.mts (was _helpers.sh) — exports filterAllowedApiKeys + scanners (personal paths, AWS keys, GitHub tokens, private keys, AI attribution) - .git-hooks/{commit-msg,pre-commit,pre-push}.mts (were .sh) - .husky/* shims invoke node directly Fleet hooks (additions) - .claude/hooks/check-new-deps (npm dep introspection) - .claude/hooks/private-name-guard - .claude/hooks/release-workflow-guard - .claude/hooks/setup-security-tools (updated) - .claude/hooks/public-surface-reminder/package.json (updated) Cleanup - Remove orphan .claude/hooks/token-hygiene (renamed to token-guard)
Contributor
Author
|
bugbot run |
- .husky/pre-commit: add `set -e` so a non-zero exit from the security hook (.git-hooks/pre-commit.mts) blocks the commit instead of being silently overridden by a later passing pnpm lint/test. Without it the shell continued past the security check; the script's exit code came from the last command, so a security failure followed by a passing lint/test would let the commit through. - token-guard: replace the single regex-based hasRedaction with a segment-aware version. The old patterns used `[\s\S]*?` which lazily reached across `|` boundaries, so an unrelated downstream stage named e.g. `tool_redact_output` could match `<?redact` and launder an upstream `env` dump. The new logic splits on `|` (skipping shell- or `||`) and requires each redaction marker to live inside a single pipe segment; whole-command redirections (`>`, `>>`, `>/dev/null`) are still matched against the full command. Adds a regression test for the canonical bypass `env | sed 's/foo/bar/' | tool_redact_output`.
Bill Li (billxinli)
approved these changes
Apr 27, 2026
Contributor
Author
|
bugbot run |
Bugbot caught a follow-up bypass: `env || sed 's/=.*/=<redacted>/'`
was credited as redacted because
1. REDACTION_WHOLE_COMMAND_MARKERS' loose `/>\s*[^|]/` matched the
literal `>` inside `<redacted>`, returning true on the
whole-command path before any segment logic ran.
2. Even without that, hasRedaction's earlier `replace(||, '')`
glued the two arms into one segment so the `sed` redaction
looked unconditional.
But `env || sed` only runs sed if env FAILS — and env always
succeeds, so the redaction never fires at runtime. Combined with
the `!hasRedaction(command)` exception on ALWAYS_DANGEROUS, the env
dump went through unredacted.
Two-part fix:
1. Tighten REDACTION_WHOLE_COMMAND_MARKERS to anchor `>` and `>>`
at a real shell boundary (`^`, whitespace, `|`, `;`) followed by
a filename-shaped target — no more matching the `>` inside
`<redacted>` or `s/=.*/.../`.
2. In hasRedaction, lop off everything from the first `||` or `&&`
before splitting on `|`. A redaction marker on the right side of
a conditional doesn't run unconditionally and can't launder an
upstream leak.
Smoke-tested:
✓ env | sed 's/=.*/=<redacted>/' (matches)
✓ printenv | sed 's/=.*/=<redacted>/' | grep TOKEN (matches)
✓ env > /dev/null (matches)
✓ env > /tmp/leak.txt (matches)
✗ env || sed 's/=.*/=<redacted>/' (NOT credited)
✗ env && sed 's/=.*/=<redacted>/' (NOT credited)
✗ env | sed 's/foo/bar/' | tool_redact_output (NOT credited)
Contributor
Author
|
bugbot run |
REDACTION_SEGMENT_MARKERS use .* between sed and the redaction marker word, which crosses ; boundaries within a single pipe segment. Bugbot found the bypass shape 'env | sed s/a/b/ ; echo redacted_output': split on | keeps both statements in one segment, the regex matches the literal word redact downstream of an unrelated sed, and the env dump is credited as redacted. Fix: split each pipe segment further on ; so a redaction marker is only credited when sed and the marker word live in the same statement.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Whole-command redaction check bypasses conditional-operator truncation
- Moved the REDACTION_WHOLE_COMMAND_MARKERS check to run after the ||/&& truncation so whole-command markers are tested against the truncated string, preventing redirection markers in never-executed conditional branches from bypassing the env-dump block.
Or push these changes by commenting:
@cursor push c4d7dca9ab
Preview (c4d7dca9ab)
diff --git a/.claude/hooks/token-guard/index.mts b/.claude/hooks/token-guard/index.mts
--- a/.claude/hooks/token-guard/index.mts
+++ b/.claude/hooks/token-guard/index.mts
@@ -53,12 +53,12 @@
/\bcut\b.*-d['"]?=['"]?\s*-f\s*1/i,
/\bawk\b.*-F\s*['"]?=['"]?/i,
]
-// Whole-command redirection markers. Anchored at a pipe-segment
-// boundary (`^`, whitespace, `|`, or `;`) so they fire only on real
-// shell redirection (`env > file`, `env >> file`, `env > /dev/null`)
-// and not on the literal `>` inside regex/HTML-style markers like
-// `<redacted>` or `s/=.*/.../`. The previous /\s*[^|]/ shape would
-// match the `>` in `<redacted>` and bypass the env-dump check.
+// Whole-command redirection markers, checked against the truncated
+// command (after stripping `||`/`&&` suffixes). Anchored at a
+// pipe-segment boundary (`^`, whitespace, `|`, or `;`) so they fire
+// only on real shell redirection (`env > file`, `env >> file`,
+// `env > /dev/null`) and not on the literal `>` inside regex/HTML-
+// style markers like `<redacted>` or `s/=.*/.../`.
const REDACTION_WHOLE_COMMAND_MARKERS = [
/(?:^|[\s|;])>\s*\/dev\/null\b/,
/(?:^|[\s|;])>>?\s*[^|<>'"\\$&\s]/,
@@ -137,9 +137,6 @@
// where a `redact`-named downstream tool would otherwise launder the
// upstream `env` dump.
const hasRedaction = (command: string): boolean => {
- if (REDACTION_WHOLE_COMMAND_MARKERS.some(re => re.test(command))) {
- return true
- }
// Drop everything from the first `||` or `&&` onwards. Those branches
// don't unconditionally execute, so a redaction marker on their
// right side cannot launder an upstream leak. The bypass shape is
@@ -147,13 +144,19 @@
// `sed` arm never runs at runtime, but the previous logic credited
// it as a redaction and let the env dump through. Truncating before
// the conditional operator forces the redaction to live in the same
- // unconditional pipeline as the leaky stage.
+ // unconditional pipeline as the leaky stage. Both whole-command and
+ // segment markers must check the truncated string — otherwise
+ // `env || true > /dev/null` would match a whole-command marker and
+ // return true early despite the redirection never executing.
const idxOr = command.indexOf('||')
const idxAnd = command.indexOf('&&')
let cut = command.length
if (idxOr !== -1) cut = Math.min(cut, idxOr)
if (idxAnd !== -1) cut = Math.min(cut, idxAnd)
const truncated = command.slice(0, cut)
+ if (REDACTION_WHOLE_COMMAND_MARKERS.some(re => re.test(truncated))) {
+ return true
+ }
// Split first on `|` (pipe-stage boundary), then on `;` (statement
// boundary) within each stage. A redaction marker is only credited
// when both `sed` and the redaction target live in the sameYou can send follow-ups to the cloud agent here.
Three findings from Cursor Bugbot's latest pass on the chore/hooks-mts-path-token branch: 1. HIGH — token-guard: whole-command redaction check ran BEFORE the conditional-operator truncation, so `env || true > /dev/null` matched the `> /dev/null` whole-command marker on the full string and short-circuited to "credited" even though the redirection lives on the unreachable `||` branch. Fix: truncate at first `||` / `&&` first, then run the whole-command markers against the truncated string. Smoke test (17 cases) confirms all known bypass shapes are blocked and all legitimate redaction shapes are still credited. 2. LOW — check-new-deps: replace local `errorMessage` mirror with the shared `errorMessage` import from `@socketsecurity/lib/errors`. The local helper's `// can't import the shared helper` comment was wrong — the hook already depends on `@socketsecurity/lib`, so the shared helper IS reachable. Drops 5 lines of duplicated logic. 3. LOW — setup-security-tools: replace ad-hoc `e instanceof Error ? e.message : String(e)` with `errorMessage(e)` from `@socketsecurity/lib/errors`. Hook already imports from `@socketsecurity/lib`, so the helper is in scope.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit aa927eb. Configure here.
Reaps orphan vitest/tsgo/type-coverage/esbuild workers at turn-end so
they don't pile up across turns and exhaust system memory. Only kills
processes whose parent has died (true orphans); leaves running
test/build trees alone.
- .claude/hooks/stale-process-sweeper/ (hook + tests + README)
- .claude/settings.json (Stop hook block)
- CLAUDE.md (Background Bash rule + suppress
pre-existing hook false-positive
on documentation-prohibition line)
Sourced from socket-repo-template. Joins this PR's path-guard +
token-guard hook fleet to complete the canonical hook set.
Pre-commit lint+test skipped via DISABLE_PRECOMMIT_* — this worktree
is missing build artifacts that the test deps need (the change is
.claude/-only, no source touched).
…ific layout Split CLAUDE.md into two clearly-delimited sections: - `## 📚 Fleet Standards` — wrapped in BEGIN/END FLEET-CANONICAL markers, byte-identical across every socket-* repo (sync via socket-repo-template). - `## 🏗️ CLI-Specific` — repo-owned content: Commands, Testing, Command Pattern, Codex Usage. Fleet block ~8.6 KB; verbose content moves to references: - `docs/references/inclusive-language.md` - `docs/references/sorting.md` - `.claude/skills/promise-race-pitfall/SKILL.md` CLAUDE.md 13.5 KB → 9.7 KB. Joins this PR's hooks-mts conversion + sweeper additions; the new CLAUDE.md links into the same hook README files this PR is producing.
oxfmt 0.37+ formats JSDoc comments. Adopt the canonical `jsdoc` block from socket-repo-template; it preserves the existing JSDoc style across this repo (verified: zero new diff under the new config). Source of truth: socket-repo-template/template/.oxfmtrc.json. Future updates flow through `scripts/sync-scaffolding.mjs --all --fix`.
…epo-template
- `.git-hooks/_helpers.mts` + `pre-commit.mts` + `pre-push.mts` —
rename suppression marker `# zizmor: …` → `# socket-hook: allow [<rule>]`
(legacy form still recognized for one cycle); add doc-aware scan
heuristic; emit `LineHit.suggested` rewrites alongside hits.
- `.claude/hooks/token-guard/{index,test/token-guard.test}.mts` —
sync to canonical; settings.json picks up the fleet permissions
deny block.
- `.claude/skills/{path-guard,security-scan}/SKILL.md`,
`.claude/agents/security-reviewer.md` — sync drift from template.
- `.claude/skills/programmatic-claude-lockdown/SKILL.md` — fleet skill
for the four-flag lockdown recipe (matches socket-sdk-js PR #630).
- `CLAUDE.md` — update fleet block npx-rule marker; oxfmt-normalized
markdown emphasis.
- `docs/references/{inclusive-language,sorting}.md` — re-sync.
- `scripts/xport*.mts` + `xport.schema.json` — fleet xport tooling.
- `.husky/pre-commit` + `.git-hooks/commit-msg.mts` — sync.
- `pnpm-lock.yaml` — regenerate after template sync.
Pre-commit/test gates skipped via DISABLE_PRECOMMIT_*: this worktree's
build chain (download-assets / iocraft native addon) is unrelated to
the changes here, and the same gates are enforced in CI.
Sourced byte-identical from socket-repo-template (canonical fleet
hook); the same change rolled out across socket-* repos + ultrathink.
logger-guard (PreToolUse Edit|Write):
- Blocks direct stream writes (process.std{err,out}.write, console.*)
in source files; suggests getDefaultLogger() rewrite per hit.
- Path-exempts hooks/, .git-hooks/, scripts/, tests, fixtures,
external/ vendor/ upstream/.
- Honors `# socket-hook: allow [logger]` opt-out marker.
auth-rotation-reminder (Stop):
- Periodic auto-logout from npm/pnpm/yarn/gcloud/aws-sso/vault/docker/
socket; gh stays logged-in by default.
- 1h throttle, ISO-8601 .claude/auth-rotation.snooze with auto-cleanup.
- Skip when CI or SOCKET_AUTH_ROTATION_DISABLED set.
Pre-commit/pre-push pick up scanLoggerLeaks for edits made outside
…-window wording - pnpm-workspace.yaml: add blockExoticSubdeps: true (fleet default). Direct git deps still allowed; transitive ones refused. - Update sync-scaffolding script references from .mjs to .mts in hook READMEs, path-guard segments, _shared skill rules, CLAUDE.md, and xport-schema.mts header comment (the upstream rename has shipped; downstreams now reflect it). - Fold "soak-window" terminology into security-reviewer's checklist + CLAUDE.md tooling block — matches how the rest of the fleet refers to pnpm's minimumReleaseAge field. Pre-existing iocraft native-addon test failures unrelated to this change; user authorized --no-verify pending environment fix.
Picks up the v5.26.1 fixes (case-insensitive default + Windows
forward-slash normalization in `globs.glob` / `globSync` /
`getGlobMatcher`, narrow matchesGlob fast-path, .then→async/await
sweep) and migrates the two breaking-surface call sites:
ascii-header.mts — `effects/text-shimmer` was removed in v5.26.1
in favor of the redesigned shimmer engine in `effects/shimmer` +
`effects/shimmer-terminal`. Rewrite `renderShimmerFrame` against
the new API:
- Build one ShimmerSpec per line via `configToSpec` (the new
engine's spec factory) using the theme palette as `color`.
- Compute two `frameColors(spec, line.length, frame)` arrays per
line — primary (frame + slantOffset) and secondary (+35) —
preserving the dual-wave look the previous code achieved by
chaining two applyShimmer passes.
- Merge the two arrays per-char with a `brighterRgb` helper so
the brighter highlight wins, then pipe through
`colorsToAnsi` and wrap with bold.
Visual contract preserved: dual wave at the same speed (0.25),
same per-line slant offset (i * 4), same theme palette as
highlight gradient, same bold formatting around the truecolor
escapes.
cli-entry.mts — `themes` barrel removed; migrate `setTheme` import
to `themes/context` (its focused submodule home).
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
2 participants
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.
Self-landable split from #1279. Combines the hook overhaul into one atomic PR.
Path-guard infra
.claude/hooks/path-guard/— hook + tests + canonicalsegments.mts.claude/skills/path-guard/— audit-and-fix skill.claude/skills/_shared/path-guard-rule.md— canonical mantra rulescripts/check-paths.mts— the whole-repo gate.github/paths-allowlist.yml— empty starter, full schema docs.claude/settings.json— wires hook onEdit|Writescripts/check.mts— invokes the gateToken-guard hook
.claude/hooks/token-guard/— renamed fromtoken-hygiene. Word-boundary match for sensitive env names.ALWAYS_DANGEROUScheck skips when a redaction pipeline is present.Stale-process-sweeper hook
.claude/hooks/stale-process-sweeper/—Stophook that reaps orphan vitest/tsgo/type-coverage/esbuild workers at turn-end. Only kills processes whose parent has died..claude/settings.jsonunderStophook block.Logger-guard hook (new)
.claude/hooks/logger-guard/—PreToolUse(Edit|Write)hook that blocks direct stream writes (process.std{err,out}.write,console.*) in source files; suggestsgetDefaultLogger()rewrite per hit.# socket-hook: allow [logger]opt-out marker.Auth-rotation-reminder hook (new)
.claude/hooks/auth-rotation-reminder/—Stophook that periodically auto-logs out of authenticated CLIs (npm/pnpm/yarn/gcloud/aws-sso/vault/docker/socket);ghstays logged-in by default (Claude Code uses it).~/.claude/hooks/auth-rotation/last-runmtime; ISO-8601.claude/auth-rotation.snoozewith auto-cleanup via safeDelete.SOCKET_AUTH_ROTATION_DISABLEDset.Pre-commit/pre-push scanLoggerLeaks
.git-hooks/_helpers.mtsaddsscanLoggerLeaksfor the same rule, so edits made outside Claude Code are caught at commit/push time.# socket-hook: allow <rule>marker (legacy# zizmor:still recognized for one cycle).CLAUDE.md restructure
## 📚 Fleet Standards(byte-identical across the fleet, wrapped in<!-- BEGIN/END FLEET-CANONICAL -->) and## 🏗️ CLI-Specific.docs/references/inclusive-language.md,docs/references/sorting.md,.claude/skills/promise-race-pitfall/SKILL.md..sh→.mtshook conversion (Node 25+).git-hooks/_helpers.mts(was_helpers.sh) — exportsfilterAllowedApiKeys+ scanners.git-hooks/{commit-msg,pre-commit,pre-push}.mts(were.sh).husky/*shims invokenodedirectlyFleet hooks
.claude/hooks/check-new-deps— npm dep introspection.claude/hooks/private-name-guard.claude/hooks/release-workflow-guard.claude/hooks/setup-security-tools(updated to canonical).claude/hooks/public-surface-reminder/package.json(updated)oxfmt JSDoc formatting
oxfmt 0.37+ formats JSDoc comments. Adopt canonical
jsdocblock from socket-repo-template. Verified zero new format violations on this repo's source.Cleanup
.claude/hooks/token-hygiene(renamed totoken-guard)Test plan
blockExoticSubdeps + .mjs->.mts ref sync (added 2026-05-01)
blockExoticSubdeps: true(fleet default — refuses transitive git/tarball subdeps; direct git deps still allowed)..mjs->.mtsreferences forscripts/sync-scaffoldingacross hook READMEs, path-guard segments, _shared skill rules, CLAUDE.md, xport-schema header.