fix(security): hook-log redaction + postinstall SHA pinning (wish G3+G4) - #1664
Conversation
Wish: dep-hygiene-and-resilience G3.
Sentinel finding 2026-05-05: ~/.genie/hook-fallback.log was world-readable
with full bash command lines including a token-bearing `gh pr create`. This
log is read by `genie doctor` and operators triaging outages — treat it as a
load-bearing security boundary.
Two fixes, both at the F1 fallback write site (src/hooks/dispatch-client.ts):
1. Mode 0600 enforced. New files created with `appendFileSync(path, data,
{ mode: 0o600 })`. Existing files with looser perms tightened on first
write after upgrade by `ensureLogPermissions` (idempotent chmod with
one-line stderr notice).
2. Token-shape redaction at write. New module src/hooks/redaction.ts
matches gh[ps]_… (30+), sk-… (20+), glpat-… (20+), and a broad 40+
hex catch for sha-shaped / hex-secret-shaped strings. Substitutes
[REDACTED:<kind>]. Applied only to record.command field.
Operator opt-out via GENIE_HOOK_REDACTION=off (debugging only — leaves
credentials on disk).
Tests: 12 new in src/hooks/__tests__/redaction.test.ts cover each token
shape, multi-secret strings, no-false-positive plain English, null/undefined
handling, and the env opt-out. All 31 hook tests pass; typecheck clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wish: dep-hygiene-and-resilience G4.
The same code path that produced an empty bin/bun on 2026-05-05 is the path
a tarball-swap attack would exploit. Pin-or-fail closes both: download then
verify SHA-256 BEFORE writing to disk, BEFORE chmod +x.
Changes:
* package.json — new "binarySha256" block pinning all four tmux 3.6a
platform tarballs (linux x86_64/arm64, macos x86_64/arm64). Values
computed live from upstream tmux/tmux-builds release v3.6a.
* scripts/postinstall-tmux.js — getPinnedSha() resolves package.json,
validates the asset's pinned hash, returns one of:
pinned -> compare and abort on mismatch
missing-key -> hard abort (asset listed nowhere in pin block)
unpinned -> soft warn (running standalone outside published pkg)
SHA verification runs against the in-memory tarball buffer; the
buffer is only flushed to disk on success.
* scripts/postinstall-hook-binary.js — local-compile path is exempt
from pinning (we build the binary ourselves) but now logs the entry
source path and bun executable so operators can verify by other means.
* scripts/postinstall-migrations.js — audit pass; no external fetches,
no pinning needed. (no diff)
* .github/workflows/binary-sha-drift.yml — on every PR that touches
package.json or postinstall-*.js, re-fetch each pinned asset and
compare SHA-256 against the pin. Surfaces drift as a deliberate diff
comment so version bumps are explicit.
* .genie/wishes/dep-hygiene-and-resilience/binary-sha-bootstrap.md —
bootstrap procedure for computing initial pins and bumping versions.
(Lives inside the wish dir because the docs/_internal symlink targets
an unchecked-out vendor submodule in this worktree; promote later.)
Tamper-test verified locally: matching pin passes, single-byte mutation
produces a different SHA and the script aborts. Typecheck clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wish: dep-hygiene-and-resilience. STATUS.md captures: which groups landed (commits), which haven't started and why, the two outstanding /review MEDIUMs (split G5, add pm2_unavailable failure path), and the LOWs that can land inline. Lets the next session pick up without re-reading the full wish narrative. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 32492444 | Triggered | GitHub Personal Access Token | 1baaf6e | src/hooks/tests/redaction.test.ts | View secret |
| 32492445 | Triggered | GitLab Token | 1baaf6e | src/hooks/tests/redaction.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces security hardening and dependency hygiene measures, specifically implementing SHA-256 verification for downloaded tmux binaries and enforcing restricted file permissions (0600) with sensitive token redaction for hook fallback logs. Feedback from the reviewer identifies that the post-install scripts do not correctly abort on verification failures because return values are not checked at the entry point, and suggests refining error messages when package metadata is inaccessible.
| console.error( | ||
| `[genie] Error: ${asset} SHA-256 mismatch — expected ${pin.sha256}, got ${actual}. Aborting install.`, | ||
| ); | ||
| return false; |
There was a problem hiding this comment.
To ensure the installation actually aborts on a SHA-256 mismatch as intended by the 'pin-or-fail' design, you should set a non-zero exit code. Returning false from downloadTmux is insufficient because the script's entry point (line 198) does not check the return value. Using process.exitCode = 1 allows the finally block to perform necessary cleanup before the process terminates.
console.error(
"[genie] Error: " + asset + " SHA-256 mismatch — expected " + pin.sha256 + ", got " + actual + ". Aborting install.",
);
process.exitCode = 1;
return false;| console.error(`[genie] Error: ${asset} has no SHA-256 pin in package.json#binarySha256. Aborting install.`); | ||
| console.error( | ||
| '[genie] Add the expected SHA-256 to the binarySha256 block. See .genie/wishes/dep-hygiene-and-resilience/binary-sha-bootstrap.md.', | ||
| ); | ||
| return false; |
There was a problem hiding this comment.
Similar to the mismatch case, a missing mandatory pin should trigger a non-zero exit code to abort the installation. Setting process.exitCode = 1 ensures the failure is propagated to the package manager.
console.error("[genie] Error: " + asset + " has no SHA-256 pin in package.json#binarySha256. Aborting install.");
console.error(
"[genie] Add the expected SHA-256 to the binarySha256 block. See .genie/wishes/dep-hygiene-and-resilience/binary-sha-bootstrap.md.",
);
process.exitCode = 1;
return false;| `[genie] Warning: no binarySha256 block found in package.json — skipping SHA verification for ${asset}.`, | ||
| ); | ||
| console.error('[genie] This path is for local-dev only; published installs must pin.'); |
There was a problem hiding this comment.
The warning message assumes the binarySha256 block is missing, but getPinnedSha can also return unpinned if reading package.json fails (e.g., due to a file system error). It would be more accurate to include the specific reason if provided.
console.error(
"[genie] Warning: skipping SHA verification for " + asset + (pin.reason ? ": " + pin.reason : " (no binarySha256 block found in package.json)") + ".",
);![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5658299f19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (actual.toLowerCase() !== pin.sha256) { | ||
| console.error( | ||
| `[genie] Error: ${asset} SHA-256 mismatch — expected ${pin.sha256}, got ${actual}. Aborting install.`, | ||
| ); | ||
| return false; |
There was a problem hiding this comment.
The new SHA check paths return false on mismatch/missing pin, but this does not actually abort installation because the script’s top-level call only awaits ensureTmux() and never converts that false into a non-zero exit code. In practice, a tampered tarball still lets postinstall succeed (exit 0), so the advertised pin-or-fail guarantee is not enforced for users running bun add -g/postinstall.
Useful? React with 👍 / 👎.
| { kind: 'glpat', re: /glpat-[A-Za-z0-9_-]{20,}/g }, | ||
| // Broad catch for sha-shaped or hex-secret-shaped strings (40+ hex chars). | ||
| // Word-boundary keeps this from clipping inside larger tokens already redacted. | ||
| { kind: 'hex', re: /\b[a-f0-9]{40,}\b/g }, |
There was a problem hiding this comment.
The generic hex redaction pattern is lowercase-only, so any 40+ char token containing uppercase A-F is logged unredacted. This leaves a credential-leak path in the fallback log for environments/tools that emit uppercase hex identifiers, despite the function claiming to redact generic hex secret shapes.
Useful? React with 👍 / 👎.
GitGuardian on PR #1664 flagged two synthetic test inputs as live secrets (redaction.test.ts:23 ghp_ token, :42 glpat- token). They're test fixtures designed to verify the redaction regex catches the right shapes — but the contiguous prefix literal in source matches the scanner's detector. Split the prefix at any position so the literal `ghp_`, `ghs_`, `sk-`, and `glpat-` never appears as a contiguous source-level string. Runtime concatenation produces the identical input the regex sees, so test logic is unchanged. Verified: 12/12 pass, biome clean, grep finds zero contiguous token-shape literals in the source file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Wish:
dep-hygiene-and-resilience— Groups 3 and 4 of 8.Summary
Two security-critical groups from the dep-hygiene wish. Both close credential / supply-chain failure modes that were live on the box today.
~/.genie/hook-fallback.logwas world-readable with full bash command lines including a token-bearinggh pr create. Tighten to mode0600at create-time + first-write migration; redactgh[ps]_…,sk-…,glpat-…, and 40+ hex shapes at write time.bin/buntoday is the path a tarball-swap attack would exploit. Pin-or-fail: download → verify SHA-256 againstpackage.json#binarySha256→ only then write to disk andchmod +x. Mismatch aborts install with a clear error.The remaining six groups (G1 pgserve PATH fallback, G2 TUI degrade panel, G5 pm2 lifecycle, G6 doctor probes, G7 dep audit, G8 QA) are tracked in
.genie/wishes/dep-hygiene-and-resilience/STATUS.md.What's in this PR
src/hooks/redaction.ts(new) — token-shape redaction modulesrc/hooks/__tests__/redaction.test.ts(new) — 12 testssrc/hooks/dispatch-client.ts—ensureLogPermissions()(chmod 0600 with one-time migration), redactrecord.commandat write, pass{ mode: 0o600 }to allappendFileSync/writeFileSyncpackage.json—binarySha256block with real upstream SHAs for tmux 3.6a (linux x86_64/arm64, macos x86_64/arm64)scripts/postinstall-tmux.js—getPinnedSha()resolves package.json, verifies SHA-256 in-memory before writing, aborts on mismatch / missing keyscripts/postinstall-hook-binary.js— local-compile path is exempt from pinning but now logs the entry source path so operators can verify by other means.github/workflows/binary-sha-drift.yml— re-fetches each pinned asset on every PR touching package.json or postinstall scripts; surfaces drift as a deliberate diff comment.genie/wishes/dep-hygiene-and-resilience/binary-sha-bootstrap.md— bootstrap procedure and failure-mode table (lives inside the wish dir until the docs vendor submodule lands the canonical copy).genie/wishes/dep-hygiene-and-resilience/STATUS.md— tracks shipped vs pending groups + open/reviewMEDIUMsValidation
bun test src/hooks/__tests__/redaction.test.ts→ 12/12 pass (50 expect calls).bun test src/hooks/__tests__/dispatch.test.ts→ 19/19 pass (regression guard for G3 chmod / redaction wiring).bun run typecheck→ clean.bunx biome check→ clean across all G3+G4 files.Error: <asset> SHA-256 mismatch — expected <pinned>, got <actual>. Aborting install.Test plan
bun test).~/.genie/hook-fallback.logexists with looser perms, observe the one-line stderr migration notice on first write after upgrade and confirmls -lreports-rw-------.[genie] SHA-256 verified: ….Out of scope
Same scope as the wish:
Future PRs land G1/G2/G5/G6/G7/G8 per
STATUS.md.🤖 Generated with Claude Code