feat(security): cherry-pick SEC-1 leak-guard CI check + pre-push hook (L1/L3/L4) - #35
Open
asachs01 wants to merge 4 commits into
Open
feat(security): cherry-pick SEC-1 leak-guard CI check + pre-push hook (L1/L3/L4)#35asachs01 wants to merge 4 commits into
asachs01 wants to merge 4 commits into
Conversation
… (SEC-1 L1/L4) (grandamenium#698) L1: .github/workflows/leak-guard.yml runs .github/scripts/leak-guard.sh on every pull_request (fork-safe, no secrets) and push to main — the server-side backstop a local pre-push hook cannot provide (covers fork PRs and UI-merges). Blocks on the operational-LEAK SHAPE (operator home paths, agent-roster+cron-schedule tables, secret-shaped tokens, dev-report artifact paths), NOT on legitimate framework convention (agent-name placeholders, lifeos test fixtures). Ships with a falsifiability test proving it FAILS on a planted leak and PASSES on the clean tree. Intended as a required status check (applied post-merge, L2). L4: broaden .gitignore so root-level *INSTALL_REPORT.md / PHASE*-REPORT.md / docs/phase-reports/ dev artifacts cannot re-track. Remediation follow-up to the 2026-07-01 fleet-metadata leak. Co-authored-by: James Goldbach <cortextos@Mac.fios-router.home> (cherry picked from commit 8267bab)
…k-guard (SEC-1 L3) (grandamenium#704) * feat(security): auto-install pre-push hook + windowed roster/cron leak-guard (SEC-1 L3) Two defense-in-depth hardenings on top of the SEC-1 leak-guard work (L1 CI scan, L2 branch protection, L4 gitignore): L3(a) — Auto-install the tracked pre-push build+test gate. `scripts/hooks/pre-push` and `scripts/setup-hooks.sh` already ship, but nothing wired the installer up, so fresh clones never got the local gate. - setup-hooks.sh is now NON-CLOBBERING: it installs only when no pre-push hook exists (or the existing one is byte-identical); if a different hook is already present it is left untouched. Safe/idempotent to re-run and never overwrites a user's own hook. - install.mjs (new section 9b) and `cortextos init` both call the installer best-effort and NON-FATAL — a hook-install failure never aborts install or init. Gated on non-Windows + repo presence + installer presence. L3(b) — Windowed roster+cron heuristic in leak-guard.sh. The same-line ROSTER_CRON_RE misses a leaked ops table that splits an agent name and its cron expression across adjacent rows. Added an awk windowed scan (WINDOW=3) that flags a roster name and a cron expression co-occurring within 3 lines, using the exact cron alternatives from the existing RE. It fires only when the same-line check did not, so the class reports at most once per file, and inherits the existing test/fixture skip. Verified no new false positives against the full tree (leak-guard --tree HEAD stays clean). Adds tests/leak-guard.test.sh cases: a multi-line roster+cron table must be flagged; a far-apart name+cron control must stay clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(security): non-clobber guard also catches a broken symlink at hook dest `[[ -e "$dest" ]]` is false for a broken symlink (it follows the link and the target is missing), so a broken-symlink pre-push would fall through the guard and get cp-overwritten — a small hole in the never-overwrite guarantee. Add `|| -L "$dest"` so a broken symlink is treated as an existing hook and left in place. Verified: no-hook installs, identical reports already-installed, different is skipped and preserved, broken symlink is skipped and preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 5a0882d)
…rator Upstream hardcoded OPERATOR_USERS=cortextos - their own maintainer's machine username, not ours. As cherry-picked, the guard was both blind to our actual leak class (would never match our real operator's path) and noisy on legitimate fixtures that happen to reuse "cortextos" as a generic placeholder (tests/sprint7-environment.test.ts, tests/unit/cli/send-telegram-normalize.test.ts). Swaps the pattern to our real identity and updates the falsifiability test's planted-leak fixture (tests/leak-guard.test.sh) to match, same split-string technique as upstream so the test file itself carries no operator-path literal.
Running the newly-cherry-picked leak-guard against our actual tree (not upstream's) surfaced two real, pre-existing issues: 1. 6 docs/superpowers/plans/*.md files leaked the real operator's home path (/Users/asachs/cortextos) in plain "Working dir:" lines. Low severity (username disclosure only - verified no secrets/credentials/ tokens/IPs in any flagged file before touching anything), but exactly the leak class this guard exists to catch. Redacted to `~/cortextos` in place - these are real working docs, worth keeping. 2. 11 docs/phase-reports/*.md files + WINDOWS_INSTALL_REPORT.md were still tracked despite this repo's own .gitignore already stating docs/ is "personal, not for public repo" - left over from before that rule was added. Content reads as upstream's own demo/test report material (references James/Luke Stevens, boris/paul/etc. placeholder agent names), not real WYRE fleet data - but the path shape alone is exactly what this guard's L4 rule targets, and a full-tree scan on every push to main would otherwise make the new required check permanently red from the moment it merges. git rm --cached (not deleted) - files stay on disk, already covered by the existing docs/ gitignore rule so they will not re-track. Verified: tests/leak-guard.test.sh passes clean, full `--tree HEAD` scan is clean.
Member
Author
|
Analyst review — cleared, independently re-verified, not just re-reading the PR body. Cloned fresh in a disposable /tmp checkout (not this working tree) and ran the actual mechanisms myself:
Read the full diff, not just the summary:
No gaps found. Good work — the mechanism catching 2 real pre-existing issues before merge is a genuinely strong argument for shipping it. Ready for Aaron's batch. |
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.
Summary
grandamenium/cortextos) commits, adapted for our fork:8267bab9(SEC-1 L1/L4 — server-side leak-guard CI check + broadened.gitignore) and5a0882d0(SEC-1 L3 — auto-installed pre-push hook + windowed roster/cron leak-guard heuristic).task_1784815034573_65702537— not a full upstream sync (fork has diverged too far: 342 local-only vs 247 upstream-only commits touching the same core files).What changed
8267bab9/5a0882d0cherry-picked as-is — only conflict was a pure-addition.gitignoreoverlap, resolved by keeping both sides.fix(security): adapt leak-guard OPERATOR_USERS to our fork's real operator— upstream hardcodedOPERATOR_USERS=cortextos(their own maintainer's machine username). As cherry-picked this was both blind to our real leak class and noisy on legitimate fixtures reusing "cortextos" as a placeholder (tests/sprint7-environment.test.ts,tests/unit/cli/send-telegram-normalize.test.ts). Swapped to our real identity; updated the falsifiability test's planted-leak fixture to match (same split-string technique so the test file carries no literal operator path).fix(security): remediate pre-existing leak-guard findings on our tree— running the guard for real (not just cherry-picking blind) surfaced:docs/superpowers/plans/*.mdfiles leaked the real operator's home path (/Users/asachs/cortextos) in plain "Working dir:" lines. Verified no secrets/credentials/tokens/IPs in any flagged file before touching anything — this is a username-disclosure-only finding, low severity. Redacted to~/cortextosin place (real working docs, worth keeping).docs/phase-reports/*.md+WINDOWS_INSTALL_REPORT.mdwere still tracked despite this repo's own.gitignorealready sayingdocs/is "personal, not for public repo" — left over from before that rule existed. Content reads as upstream's own demo/test-report material (references James/Luke Stevens,boris/paul/etc. placeholder agent names), not real WYRE fleet data.git rm --cached(not deleted — files stay on disk, already covered by the existingdocs/ignore rule).Why this matters for review
The two "remediate" findings above are the whole argument for shipping this: the mechanism caught 2 real pre-existing issues before it was even merged. Boss is aware of the operator-path finding and will flag it to Aaron directly (his own path was in the leak).
Test plan
tests/leak-guard.test.sh(the mechanism's own falsifiability test) — PASS.github/scripts/leak-guard.sh --tree HEAD(full tree scan, same mode used on push-to-main) — cleanpassword=/token=/secret=shapes) across every flagged file — zero hits, confirmed before any file was touchednpm ci+npm run build— cleannpm test— 13 test files / 35 tests fail, confirmed pre-existing on unmodifiedorigin/main(unrelated to this change — hooks/bus dispatch tests, not leak-guard-adjacent). Not introduced by this PR; not fixed by this PR either, out of scope.🤖 Generated with Claude Code