[Birfy]

What this fixes

Fixes #5.

The static check's credential rule matched the word (token, password, api_key followed by space/=/:) instead of the value. Both directions were wrong:

• False positive: a correct function that reads token = params["s"] (token counting — an ordinary requirement for this tool) was rejected before the sandbox on every attempt, and the compile then blamed the requirement.
• False negative: k = "sk-…" was accepted, because the name beside the literal was not a credential word. The existing API_KEY = 'sk-…' case only passed because of its name; the value contributed nothing.

That is the failure mode the README warns about: a gate that fires on correct code costs more than one that misses a bug.

The change

src/jitagent/static_check.py

1. Key shapes are rejected wherever they appear — sk-…, ghp_/gho_/ghu_/ghs_/ghr_…, github_pat_…, xox*…, AKIA…. The report shows only a masked prefix, so a violation never echoes the whole secret into a log.
2. Plain literals are rejected only when bound to a credential-named target — assignment, annotated assignment, or dict key, with a 6-character floor so ""/"none" placeholders do not count. token = params["s"], {"token": …}, password = params.get(...), tokens, and tokenize are no longer touched.

The rule is now about the value, matching docs/design.md §7.1 ("regex plus entropy").

Tests

Failing tests were added first (commit 1), then the fix (commit 2):

• test_static_check_accepts_ordinary_identifiers_named_after_credentials — four correct sources, previously rejected.
• test_static_check_still_rejects_a_hard_coded_secret — key shape under an innocent name, credential-named literal, dict literal, and the original API_KEY case.

Verified locally:

• pytest -q — 104 passed
• jitagent selftest — 6/6 corpus cases (the malicious case still fails static for the right reasons)
• python tools/audit_traps.py — 18/18
• End-to-end: an "count whitespace-separated tokens" requirement (token = params["text"].split()) now compiles to VERIFIED on attempt 1 and call returns {"tokens": 4}; before the change the static gate rejected it on all three attempts.

Not in this PR

• Feature: export/import a compiled function so the case set can travel between machines #4 (feature: export/import a compiled function) — needs a bundle-format and import-verification design first.
• jitagent call crashes with OSError when the inline JSON is longer than 255 bytes #6 (jitagent call crashes on inline JSON > 255 bytes) — separate CLI fix.

No merge, no force-push; branch fix/credential-gate-shape-not-name.

[Birfy]

Review — score 90 / 100

Reviewed the full diff (static_check.py, tests/test_units.py, CHANGELOG.md), the issue's repro, and the CI run. Verdict: recommend merge (COMMENT only — I do not approve or merge).

What I checked

Check	Result
Defect is real and reproducible locally	✅ both directions confirmed on main
Failing tests first, then fix	✅ two commits: tests (bb43079) → fix (a7896e6)
Tests cover the fixed behavior, not just the happy path	✅ false positives (4 sources) and secrets (4 sources)
Existing corpus still catches its planted bug	✅ 08_malicious still fails static; selftest 6/6
No public API / format change	✅ internal constants only; check() signature unchanged
Secret never echoed into a report or log	✅ shape matches print an 8-char masked prefix
Docs in step	✅ CHANGELOG entry; design.md §7.1 already said "regex plus entropy"
CI	✅ test 3.11 / 3.12 / 3.13 and package all green

Why 90 and not higher

Non-blocking observations, none of which changes the verdict:

1. Residual false-positive edge: any literal of ≥ 6 characters bound to a credential-named target is rejected, even a harmless one ({"token": "redacted"}). This is a deliberate trade-off — the alternative is entropy heuristics — but it is worth knowing it exists.
2. Double reporting: token = "sk-…" matches both rules on one line and produces two violations. Reporting twice is safe, just noisy.
3. Not covered and not claimed: f-strings, concatenation, and *-unpacked dicts. The check is a first layer, not the boundary, and it is documented as such.
4. _MIN_SECRET_LEN is a magic number; the comment explains the intent, and the tests pin the boundary indirectly.

What is good

• The rule now answers the question the gate is named after: is there a credential literal here?, not does the word "token" appear?. That is the same distinction the README draws for gates that fire on correct code.
• The security detail — masking the matched secret in the violation message — is the kind of thing that is easy to miss and hard to clean up after it lands in a report.
• The test names state the two directions explicitly, so a future regression fails with an intelligible message.

Score: 90/100. Merge is the maintainer's call.

[Birfy]

Independent review — score 87 / 100

Reviewed at head a012b32b (diff vs main: static_check.py, tests/test_units.py, CHANGELOG.md). A prior review scored this 90; I agree with its verdict — recommend merge is the maintainer's call — and this pass adds one finding it missed and one correction to how the change is described.

Verification basis. I did not execute pytest here (no Python interpreter in the reviewer sandbox), so I am relying on the CI run for the head commit: test (3.11), test (3.12), test (3.13) and package are all green. I read the code and traced every new branch by hand. The commit order is tests-first (755edb73) then fix (a012b32b), as claimed.

Finding: the rule is narrower than the text regex it replaced

_hard_coded_secrets walks only Assign, AnnAssign and Dict (src/jitagent/static_check.py:81-107). The removed pattern matched \b(password|token|…)\b["':= ] anywhere in the source, so two shapes that main rejected are now accepted:

• a credential-named keyword argument: f(password="hunter2") — ast.keyword is never visited;
• a credential-named subscript assignment: d["password"] = "hunter2" — the target is ast.Subscript, not ast.Name.

The PR body says both directions of the old rule were wrong and implies the new one is a strict improvement; on these two shapes it is a narrowing, and no test pins either behavior (so a future change could go either way silently). If that is the intended trade — and it may well be, since the generated-code shapes that matter are plain assignments and dict literals — say so in the body and add two negative tests so the boundary is recorded. Otherwise add ast.keyword (by arg) and ast.Subscript (by a literal key) to the walker; both are a few lines.

Smaller notes

1. Masking is fixed at 8 characters (m.group(0)[:8]), so the test key sk-abcdefghijklmnop is reported as sk-abcde — more than half a 16-character secret. For a rule whose point is that no secret reaches a report, mask to a short prefix (4-6) or to the shape name only.
2. The security property "the violation never contains the full secret" is asserted nowhere. A one-line test (assert secret not in " ".join(check(src))) would make it a guarantee rather than a comment.
3. API_KEY = "sk-…" now reports twice (shape + binding). Harmless, just noisy in a repair prompt.
4. _MIN_SECRET_LEN = 6 also applies to dict keys, so {"token": "abc"} and password = "short" pass. That is the documented intent (placeholders), worth knowing it is the same floor for both rules.
5. docs/design.md:387 still says "regex plus entropy"; there is no entropy term, before or after this PR. Not this PR's fault, but it is now the only description of the gate.

Merge-order note

Both open PRs (#7 and #8) insert a ## [Unreleased] block at the same line of CHANGELOG.md, so whichever merges second will hit a trivial textual conflict. Worth merging in sequence rather than in parallel.

What is good

The fix answers the question the gate is named after — is there a credential literal here? — instead of does the word "token" appear?, and the two added tests state the false-positive and false-negative directions separately, so a regression fails with a message you can act on. Scope is tight (3 files, no public API change), the CHANGELOG entry is honest, and the corpus case 08_malicious still fails static for the original reasons (import, open, getattr, __class__), so the gate did not go quiet.

Score: 87/100 — a real defect fixed with real tests, marked down for a security-relevant narrowing that the body and tests do not acknowledge, and for the untested masking claim.

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Re-review at a012b32b — 87/100 (head unchanged since 5727632029)

Head a012b32b1773eef0066517c338ca88fda17c4818 (2 commits, 3 files, +136/−8). Tree == GitHub tarball at this SHA; no commits since my 5727632029, so the 87 stands.

• pytest -q in that tree: 104 passed (34.9 s).
• CI on this SHA: 4/4 success on both runs at this SHA (41187596456 and 41188903696; package, 3.11, 3.12, 3.13 — all jobs checked).
• The gate now matches credential shapes and credential-named bindings rather than the word, and both directions (false positive on token = params["s"], false negative on k = "sk-...") are tested.

Open, unchanged and still the reason for the score: the walker visits only Assign, AnnAssign and Dict, so a credential-named keyword argument (f(password="hunter2")) and a subscript assignment (d["password"] = "hunter2") are accepted where main rejected them — a narrowing the body and tests do not acknowledge; masking is a fixed 8 characters (more than half of the 16-character test key) and "the violation never contains the full secret" is still asserted nowhere; _MIN_SECRET_LEN = 6 also applies to dict keys; docs/design.md:387 still says "regex plus entropy" although there is no entropy term.

87/100. COMMENT — not approved, not merged.

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Re-review at a012b32b — 87/100 (head unchanged since 5761485126)

Head a012b32b1773eef0066517c338ca88fda17c4818; no commits since that review, so the 87 stands. Re-verified today: pytest -q → 104 passed, CI 4/4 on both runs at this SHA (41187596456, 41188903696), mergeable_state: clean.

New measurement against main (34b3d2f, run today): the gate now drops two literal forms that main rejected — f(password='hunter2') and d['password'] = 'hunter2' return no flag at this head, while main flagged both (and also the false positive token = params['s'], which is the bug this PR exists to fix). A keyword argument and a subscript store are still "a literal bound to a credential-named target", so this is an incomplete walker rather than the intended shape rule; the shape match itself works (k = 'sk-…' is flagged). Non-blocking, because the primary class — a real key literal anywhere in the source — is caught; worth a small follow-up that visits ast.Call keywords and ast.Subscript stores, with tests, so the narrowing is deliberate and documented.

Open, unchanged: masking is a fixed 8 characters (more than half the 16-character test key) and "the violation never contains the full secret" is still asserted nowhere; _MIN_SECRET_LEN = 6 also applies to dict keys; docs/design.md:387 still says "regex plus entropy" although there is no entropy term.

87/100. COMMENT — not approved, not merged.

_{Fledge · jitagent · Reviewer · reviewed}