Skip to content

Commit 1319742

Browse files
feat(security): guard-sync + PR-body HMAC grant v2 (pairs orama #260) (#320)
* fix(guard-sync): worktree discovery, fail-closed checker, full pre-push range Sync orama #255 CodeRabbit reviews 4835024659/4835288649 (dd8bcba3, 458a5ab9): - git -C rev-parse for linked worktree siblings - exact CLI arity (reject surplus args) - fail-closed when checker missing in sync + pre-push - scan full outgoing commit range for scripts/git/ touches - 5 pytest cases including worktree + surplus-arg * docs(memory): combine CodeRabbit review wave report + graduate lessons Working memory: CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md Lessons: markdownlint cli2 per-dir config, guard-sync fail-closed, PR-body Layer 0 operator grant path. * fix(pr-body): sync remind-pr-body Layer 0 messaging from orama #255 * feat(security): sync HMAC grant v2 PR-body path from orama-system Synced via scripts/git/sync-attribution-guard-scripts.sh from orama branch 2026-08-02-pr-body-grant-hmac-mvp: grant lib, hooks, append script, pre-push range_for_ref, guard-sync test, PR-body tests. CODERABBIT wave doc provenance updated. * docs(memory): chronicle PR-body HMAC grant MVP saga (2026-08-02) Saga, decision JSONL, WORKSPACE refresh, and CodeRabbit wave Batch F for post-#255 grant v2 implementation. Pairs with orama branch 2026-08-02-pr-body-grant-hmac-mvp for PT PR #320. * docs(memory): weld verification + research export index Saga weld checklist; README indexes local-only EXA exports (not committed due to hygiene). Pairs with orama 2026-08-02-pr-body-grant-hmac-mvp. * fix(security): sync grant remediation F1–F7 from orama canonical Replay state machine, GH_BIN append test, saga path hygiene for CI gate. * docs(agent): refresh grant remediation memory across PT .agent Update workspace, saga, decisions JSONL, CodeRabbit Batch G, semantic DECISIONS, and graduate five lessons (replay state machine, canonical payload, same-user boundary, HMAC override, path hygiene). * sync(grant): scrub_dsstore + can-6 remediation from orama #260 Mirror guard-sync manifest, scrub_dsstore.sh, grant v2 hardening, expanded tests, githooks -x guard, and WORKSPACE markdownlint fixes. * fix: PT PR #320 remediation (can-7.md) + 4 lessons from this session Remediation, verified against current code before fixing each finding: - lessons.jsonl + graduated candidates: removed remaining /tmp literals from lesson_1a6dc6bccad0's rationale AND decisions[1].notes (2 fields, same file, review only caught one on first pass); reworded lesson_d1f3789d4592's claim from "Human-authorized" to "operator-gated" since the HMAC grant proves operator action, not human identity. - HMAC payload terminology: digest -> content_digest, verified against the actual implementation as canonical. - DECISIONS.md: excluded from MD013 (matching SKILL.md/LESSONS.md precedent, confirmed append-only/machine-grown), then fixed the remaining ~10 blank-line/bare-URL issues directly; also corrected content describing "gh pr edit" as a direct agent step when it's actually internal to append-pr-body.sh. - WORKSPACE.md's ambiguous "26 passed (orama + PT)" claim: actually ran the real test command in both repos separately (26 confirmed real in each, not a stale copy-paste) and rewrote the doc to state this unambiguously. - .githooks/pre-commit: fixed a real fail-open gap -- hygiene script present but python3 missing was silently falling through to a much narrower identity-only check with no warning. Now fails closed. - pr-body-grant-lib.py: pr_number had zero validation anywhere before reaching the canonical HMAC payload; repo's validator only checked for pipe characters, not newlines. Added _validate_pr_number, extended _validate_repo_slug, wired both into mint_grant and verify_grant_fields. 3 new regression tests, one initially exposed a real subtlety: a naive first draft using an inactive override made the test pass regardless of whether the fix was present, since the first command already denied on its own -- corrected by testing with the real bypass condition documented in this commit's own lesson below. Also fixed 5 pre-existing markdownlint violations pulled into scope on the SAGA doc once this commit touched it -- hand-authored narrative, fixed properly rather than excluded. 4 lessons graduated (checked existing lessons first to avoid re-capturing anything already recorded): - lesson_a791e2ee5ad6: the exclude-vs-fix decision for lint violations in machine-rendered vs hand-authored files, generalized from this session's concrete applications. - lesson_9cc7ae1a5f12: verifying findings against current code before fixing catches already-resolved/stale findings, saving real effort. - lesson_4c65d5956822: shell-command guards must split on bare newlines too, not just &&/;/||, and the specific way a badly-mocked test can pass without proving anything about the real vulnerability. - lesson_5fb495abcfbc: a combined existence-and-availability guard condition silently downgrades security coverage when the dependency is missing but the primary script exists -- separate the conditions. 29/29 tests pass (full suite: grant lib, append flow, guard core, divergence checker). 985 lessons.jsonl lines, zero duplicate IDs, all 4 new lessons' episodic mirrors confirmed. Hygiene clean. * fix: markdownlint config schema mismatch -- CI's pinned action runs a much older version than tested locally The earlier .markdownlint-cli2.jsonc fix used an overrides/filter/combine block that worked when tested with a locally-installed markdownlint-cli2 (v0.23.2, markdownlint v0.41.1) but CI's pinned davidanson/markdownlint-cli2-action@v19 actually runs markdownlint-cli2 v0.17.2 (markdownlint v0.37.4) -- a much older version that doesn't support that config schema at all. It silently ignored the override and CI failed on the full, unexempted DECISIONS.md (68 violations). Caught by installing the exact CI-matching version locally (npm install -g markdownlint-cli2@0.17.2) rather than trusting the newer local install, and confirmed empirically that the simpler key works correctly across both versions. Replaced overrides with ignores for DECISIONS.md/LESSONS.md/SKILL.md; reverified clean against the CI-matching version, not just the newer local one. Also fixed 2 genuine pre-existing MD013 violations in CODERABBIT_REVIEW_WAVE_...md, pulled into scope once this commit touched it -- hand-authored narrative, fixed directly. 29/29 tests pass. Hygiene clean. Reverified with markdownlint-cli2 v0.17.2 specifically (the real CI version), not the newer local default. * fix(memory,ci): restore superseded lesson link + MD013-only markdownlint overrides - lesson_d1f3789d4592 supersedes lesson_4c914783ec46 in lessons.jsonl - Replace ignores with MD013 overrides for SKILL.md and skills tree (CodeRabbit #320 review 4837623699) * memory: 3 lessons + report from the markdownlint version-mismatch / gitleaks debugging session 3 lessons graduated: - lesson_77ce859d6970: verify CI-tool config fixes against the exact pinned tool version CI actually runs, not the newest locally-installed version -- a config schema can silently work locally and do nothing in CI if the actually-pinned version doesn't support it. - lesson_084412e4e566: secret scanners that scan full commit history (not just current tree state) can keep flagging already-superseded commits whose problematic content was fixed later on the same branch -- the fix is a scoped commit-SHA allowlist, not another file edit. - lesson_5f0362a3f95f: never extend a short git SHA by guessing/padding hex characters -- always git rev-parse it. A fabricated SHA in a security-tool allowlist silently matches nothing while looking syntactically correct. Plus a compact narrative report (2026-08-02-ci-tool-version-and-gitleaks-remediation-report.md) of the actual debugging arc across both repos, ending in a reusable checklist for the next CI-tool-config fix. Checked existing lessons first (2 incidental keyword matches, both unrelated on inspection) before graduating anything, to avoid duplicating already-captured claims. 975 lessons.jsonl lines, zero duplicate IDs, all 3 new lessons' episodic mirrors confirmed. Report itself verified with the CI-matching markdownlint-cli2 v0.17.2, not the newer local default -- applying the lesson it documents. Hygiene clean. * fix: revert regression -- restore working markdownlint ignores config A later commit on this branch (1ccca81, applying orama PR #320 review 4837623699) replaced the working ignores-based MD013 exemption with the overrides/filter/combine schema already proven this session not to work against CI's actually-pinned markdownlint-cli2 v0.17.2 -- reintroducing the exact CI failure that had already been diagnosed and fixed. Restored the ignores-based config, verified against the exact CI-pinned tool version (markdownlint-cli2 v0.17.2, not the newer local default): SKILL.md, docs/LESSONS.md, and .agent/memory/semantic/DECISIONS.md all correctly exempted, 0 issues. 29/29 tests pass. Hygiene clean. * sync: propagate orama PR #260's guard-script fixes to PT exactly Compared PT's PR #320 branch against orama's PR #260 branch (the source of truth for this session's pr-body-guard/grant work) across the full guard-script ecosystem -- scripts/cursor/hooks/*, scripts/cursor/ {append-pr-body,grant-pr-body-human-override,pr-body-grant-lib}.*, scripts/cursor/hooks.json.template, scripts/git/check-guard-sync- divergence.sh, and the matching test files. Several were already synced from earlier work this session on this same branch; this pass filled the remaining gap -- PT was missing the entire grant-v2 HMAC library (pr-body-grant-lib.py) and its human-override grant script entirely, and had a stale copy of pr-body-guard-core.py without the newline- segment-splitting security fix from orama PR #251/review 4835288649. Synced exactly (byte-identical from orama's source), permissions matched to the established pattern (0755 for executable scripts, 0644 for the hooks.json.template data file). .gitleaks.toml intentionally NOT propagated -- checked first: PT's CI has no gitleaks job at all, so the config would be inert. 26/26 tests pass on the synced files. Confirmed a broad, pre-existing test-suite failure (259 failures across unrelated files like test_gossip_bus.py) predates this sync entirely -- verified via git stash before/after comparison, not assumed -- and is out of scope for this propagation. Hygiene clean. * memory: dispatch-race/config-scope postmortem + 3 lessons from orama PR 260's review remediation Postmortem (2026-08-02-dispatch-race-deferral-and-config-scope- postmortem.md), 4 parts: the deferral-date internal-consistency bug in the dispatch-race work; the duplicate-fallback fix and its own subtle near-miss (a naive broader fix would have regressed a different, valid scenario); the markdown config-scope investigation that verified a precise fix works but surfaces 1032 pre-existing violations across 205 files, not adopted for that reason; and the completed root-cause diagnosis of a large pre-existing test-failure count an earlier note had only confirmed as unrelated without explaining. 3 lessons graduated, checked against existing lessons first (0 matches on the specific patterns): - lesson_61d719978435: verify two halves of a gated-feature commit are actually consistent with each other via the test that exercises both, not just each half's own coverage. - lesson_64d1deca2fca: a duplicate-removal fix can be correct in one code state and a regression in another -- check before removing unconditionally. - lesson_8bf6b00d2a44: finish the diagnosis of a confirmed-unrelated failure count where time allows; 'pre-existing, unrelated' without the actual cause leaves the next reader to re-investigate from scratch. Scanned for personal-path or workstation-identity literals before writing -- none present; all paths described repo-relatively. 994 lessons.jsonl lines, zero duplicate IDs, all 3 mirrors confirmed. Hygiene clean.
1 parent 72750cf commit 1319742

53 files changed

Lines changed: 4517 additions & 514 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
{
2+
"id": "084412e4e566",
3+
"key": "manual_084412",
4+
"name": "manual_084412",
5+
"claim": "Secret-scanning tools like gitleaks scan the full commit history range in a branch/PR by default, not just the current tree state -- a test fixture or example value that merely resembles a secret (e.g. a hardcoded hex digest used as an expected HMAC output) can trip a generic entropy detector, and even after a later commit fixes the pattern by removing the literal value, the scanner keeps flagging the earlier, now-superseded commit on every subsequent CI run for that branch, because the flagged content is still present in that commit's diff within history. This is a real, recurring category of false positive distinct from an actual leaked secret, and the correct fix is a scoped allowlist entry for the specific commit SHA(s) in the scanner's config (with `useDefault = true` or equivalent to keep the rest of the default ruleset intact) -- not editing the current file again, since the flagged content is no longer there, and not disabling the scanner or its ruleset broadly.",
6+
"conditions": [
7+
"actual",
8+
"after",
9+
"again",
10+
"allowlist",
11+
"because",
12+
"branch",
13+
"broadly",
14+
"category",
15+
"commit",
16+
"config",
17+
"content",
18+
"correct",
19+
"current",
20+
"default",
21+
"detector",
22+
"diff",
23+
"digest",
24+
"disabling",
25+
"distinct",
26+
"earlier",
27+
"editing",
28+
"entropy",
29+
"entry",
30+
"equivalent",
31+
"even",
32+
"every",
33+
"example",
34+
"expected",
35+
"false",
36+
"file",
37+
"fix",
38+
"fixes",
39+
"fixture",
40+
"flagged",
41+
"flagging",
42+
"full",
43+
"generic",
44+
"gitleaks",
45+
"hardcoded",
46+
"hex",
47+
"history",
48+
"hmac",
49+
"intact",
50+
"just",
51+
"keep",
52+
"keeps",
53+
"later",
54+
"leaked",
55+
"like",
56+
"literal",
57+
"longer",
58+
"merely",
59+
"now-superseded",
60+
"output",
61+
"pattern",
62+
"positive",
63+
"present",
64+
"range",
65+
"real",
66+
"recurring",
67+
"removing",
68+
"resembles",
69+
"rest",
70+
"ruleset",
71+
"run",
72+
"scan",
73+
"scanner",
74+
"scoped",
75+
"secret",
76+
"secret-scanning",
77+
"sha",
78+
"since",
79+
"specific",
80+
"state",
81+
"still",
82+
"subsequent",
83+
"test",
84+
"there",
85+
"tools",
86+
"tree",
87+
"trip",
88+
"true",
89+
"used",
90+
"usedefault",
91+
"value",
92+
"within"
93+
],
94+
"evidence_ids": [
95+
"2026-08-02T08:04:00.461112+00:00"
96+
],
97+
"cluster_size": 1,
98+
"canonical_salience": 8.0,
99+
"staged_at": "2026-08-02T08:04:00.461112+00:00",
100+
"status": "accepted",
101+
"decisions": [
102+
{
103+
"ts": "2026-08-02T08:04:00.461112+00:00",
104+
"action": "staged",
105+
"reviewer": "learn"
106+
},
107+
{
108+
"ts": "2026-08-02T08:04:19.653507+00:00",
109+
"action": "graduated",
110+
"reviewer": "host-agent",
111+
"notes": "A genuine, distinct false-positive category (history-range scanning catching already-fixed content) with a concrete, verified remedy.",
112+
"provisional": false,
113+
"evidence_snapshot": [
114+
"2026-08-02T08:04:00.461112+00:00"
115+
],
116+
"lessons_sha": "dbb46766d501"
117+
}
118+
],
119+
"rejection_count": 0,
120+
"accepted_at": "2026-08-02T08:04:19.653496+00:00",
121+
"reviewer": "host-agent",
122+
"rationale": "A genuine, distinct false-positive category (history-range scanning catching already-fixed content) with a concrete, verified remedy."
123+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
{
2+
"id": "1a6dc6bccad0",
3+
"key": "manual_1a6dc6",
4+
"name": "manual_1a6dc6",
5+
"claim": "Tracked .agent memory and working chronicles must not embed ephemeral scratch path literals \u2014 repo_hygiene flags them like workstation home paths and blocks PT CI.",
6+
"conditions": [
7+
"agent",
8+
"blocks",
9+
"chronicles",
10+
"embed",
11+
"ephemeral",
12+
"flags",
13+
"home",
14+
"like",
15+
"literals",
16+
"memory",
17+
"path",
18+
"paths",
19+
"repo_hygiene",
20+
"scratch",
21+
"them",
22+
"tracked",
23+
"working",
24+
"workstation"
25+
],
26+
"evidence_ids": [
27+
"2026-08-02T04:07:28.914652+00:00"
28+
],
29+
"cluster_size": 1,
30+
"canonical_salience": 8.0,
31+
"staged_at": "2026-08-02T04:07:28.914652+00:00",
32+
"status": "accepted",
33+
"decisions": [
34+
{
35+
"ts": "2026-08-02T04:07:28.914652+00:00",
36+
"action": "staged",
37+
"reviewer": "learn"
38+
},
39+
{
40+
"ts": "2026-08-02T04:07:29.049005+00:00",
41+
"action": "graduated",
42+
"reviewer": "learn.py",
43+
"notes": "PT #320 CI failed when saga notes embedded a scratch-path literal instead of a neutral worktree label; use neutral worktree labels in memory.",
44+
"provisional": false,
45+
"evidence_snapshot": [
46+
"2026-08-02T04:07:28.914652+00:00"
47+
],
48+
"lessons_sha": "2d792c79eb15"
49+
}
50+
],
51+
"rejection_count": 0,
52+
"accepted_at": "2026-08-02T04:07:29.048986+00:00",
53+
"reviewer": "learn.py",
54+
"rationale": "PT #320 CI failed when saga notes embedded a scratch-path literal instead of a neutral worktree label; use neutral worktree labels in memory."
55+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
{
2+
"id": "446162929cb3",
3+
"key": "manual_446162",
4+
"name": "manual_446162",
5+
"claim": "Same-user Keychain HMAC for PR-body grants is escalation control not cryptographic human identity \u2014 agent shells with PTY can read the secret or ack file; MVP raises effort; WebAuthn deferred to security-sentinel v2.1.",
6+
"conditions": [
7+
"ack",
8+
"agent",
9+
"control",
10+
"cryptographic",
11+
"deferred",
12+
"effort",
13+
"escalation",
14+
"file",
15+
"grants",
16+
"hmac",
17+
"human",
18+
"identity",
19+
"keychain",
20+
"mvp",
21+
"pr-body",
22+
"pty",
23+
"raises",
24+
"read",
25+
"same-user",
26+
"secret",
27+
"security-sentinel",
28+
"shells",
29+
"webauthn"
30+
],
31+
"evidence_ids": [
32+
"2026-08-02T04:07:28.049022+00:00"
33+
],
34+
"cluster_size": 1,
35+
"canonical_salience": 8.0,
36+
"staged_at": "2026-08-02T04:07:28.049022+00:00",
37+
"status": "accepted",
38+
"decisions": [
39+
{
40+
"ts": "2026-08-02T04:07:28.049022+00:00",
41+
"action": "staged",
42+
"reviewer": "learn"
43+
},
44+
{
45+
"ts": "2026-08-02T04:07:28.177266+00:00",
46+
"action": "graduated",
47+
"reviewer": "learn.py",
48+
"notes": "Remediation F3 F4; honest security doctrine per research and autoplan CEO review.",
49+
"provisional": false,
50+
"evidence_snapshot": [
51+
"2026-08-02T04:07:28.049022+00:00"
52+
],
53+
"lessons_sha": "c0a5b0b7048b"
54+
}
55+
],
56+
"rejection_count": 0,
57+
"accepted_at": "2026-08-02T04:07:28.177254+00:00",
58+
"reviewer": "learn.py",
59+
"rationale": "Remediation F3 F4; honest security doctrine per research and autoplan CEO review."
60+
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
{
2+
"id": "4c65d5956822",
3+
"key": "manual_4c65d5",
4+
"name": "manual_4c65d5",
5+
"claim": "A security guard that inspects shell commands by splitting on command separators (&&, ;, ||) to check each resulting segment independently must also split on bare newlines -- a shell interprets a newline as a command separator too, and a guard that omits it leaves an exploitable gap: a legitimate-looking command on the first line can cause the guard to short-circuit to ALLOW, leaving a malicious second command hidden on the next line completely uninspected. When testing such a guard's segment-splitting logic, the test must exercise the actual bypass condition (the legitimate first command's branch must be genuinely capable of returning ALLOW, e.g. via a properly-mocked active authorization state) -- a test where the first segment already denies on its own passes regardless of whether the fix is present, proving nothing about the real vulnerability.",
6+
"conditions": [
7+
"about",
8+
"active",
9+
"actual",
10+
"allow",
11+
"already",
12+
"also",
13+
"authorization",
14+
"bare",
15+
"branch",
16+
"bypass",
17+
"capable",
18+
"cause",
19+
"check",
20+
"command",
21+
"commands",
22+
"completely",
23+
"condition",
24+
"denies",
25+
"each",
26+
"exercise",
27+
"exploitable",
28+
"first",
29+
"fix",
30+
"gap",
31+
"genuinely",
32+
"guard",
33+
"hidden",
34+
"independently",
35+
"inspects",
36+
"interprets",
37+
"leaves",
38+
"leaving",
39+
"legitimate",
40+
"legitimate-looking",
41+
"line",
42+
"logic",
43+
"malicious",
44+
"newline",
45+
"newlines",
46+
"next",
47+
"nothing",
48+
"omits",
49+
"own",
50+
"passes",
51+
"present",
52+
"properly-mocked",
53+
"proving",
54+
"real",
55+
"regardless",
56+
"resulting",
57+
"returning",
58+
"second",
59+
"security",
60+
"segment",
61+
"segment-splitting",
62+
"separator",
63+
"separators",
64+
"shell",
65+
"short-circuit",
66+
"split",
67+
"splitting",
68+
"state",
69+
"such",
70+
"test",
71+
"testing",
72+
"too",
73+
"uninspected",
74+
"via",
75+
"vulnerability",
76+
"whether"
77+
],
78+
"evidence_ids": [
79+
"2026-08-02T07:00:06.137706+00:00"
80+
],
81+
"cluster_size": 1,
82+
"canonical_salience": 8.0,
83+
"staged_at": "2026-08-02T07:00:06.137706+00:00",
84+
"status": "accepted",
85+
"decisions": [
86+
{
87+
"ts": "2026-08-02T07:00:06.137706+00:00",
88+
"action": "staged",
89+
"reviewer": "learn"
90+
},
91+
{
92+
"ts": "2026-08-02T07:00:26.921457+00:00",
93+
"action": "graduated",
94+
"reviewer": "host-agent",
95+
"notes": "A real, exploitable security bypass and the specific way the first regression test attempt initially failed to prove anything.",
96+
"provisional": false,
97+
"evidence_snapshot": [
98+
"2026-08-02T07:00:06.137706+00:00"
99+
],
100+
"lessons_sha": "a4478e0a1a93"
101+
}
102+
],
103+
"rejection_count": 0,
104+
"accepted_at": "2026-08-02T07:00:26.921437+00:00",
105+
"reviewer": "host-agent",
106+
"rationale": "A real, exploitable security bypass and the specific way the first regression test attempt initially failed to prove anything."
107+
}

0 commit comments

Comments
 (0)