diff --git a/assets/agents/adversary/graph.yaml b/assets/agents/adversary/graph.yaml index b269829..277a34c 100644 --- a/assets/agents/adversary/graph.yaml +++ b/assets/agents/adversary/graph.yaml @@ -205,7 +205,11 @@ nodes: confirm the change makes the criterion true, confirm the test targets BEHAVIOR (not implementation echo), use ast_grep for structural claims. Watch for gamed evidence: a test weakened/deleted so it passes, hardcoded - values shaped to the criterion's example, TODO/stub presented as done — + values shaped to the criterion's example, TODO/stub presented as done, a + mock expectation whose count is relaxed (`AnyTimes`, `MinTimes`, ignored + request body) AND whose matcher is permissive (`gomock.Any()`, + `mock.Anything`) — it proves a call happened, never that it carried the + criterion's values, so it is not a proving test — those are PARTIAL or DIVERGED with the gaming named in the complaint. Retry / redelivery / idempotency demands: before you mark a criterion PARTIAL diff --git a/assets/skills/code-review/SKILL.md b/assets/skills/code-review/SKILL.md index d82a2be..940af50 100644 --- a/assets/skills/code-review/SKILL.md +++ b/assets/skills/code-review/SKILL.md @@ -152,6 +152,21 @@ Named adequacy anti-patterns — each is a finding even when coverage looks gree - **Leaky fixture** — an integration test against a shared/persistent database inserts fixture rows directly with no paired cleanup (deferred delete/teardown/transaction rollback). The test passes today and leaves a primary-key landmine for the next run. +- **Struct-typed args against an inferred schema** — when a struct drives an inferred wire schema + (MCP go-sdk, OpenAPI/JSON-schema generators, serde-derived schemas), every field without + `omitempty`/pointer/`Option` becomes schema-REQUIRED. Cross-check the optionality the doc + strings promise ("X as an alternative to Y") against the tags — a documented-optional field + without the omission marker is a live validation bug, not a style nit. And tests that build the + request from the typed struct serialize EVERY field (zero values included), so they can never + exercise the required-ness check: demand raw-JSON / map-typed tests that OMIT each + documented-optional field. +- **Relaxed call count with a permissive matcher** — a mock expectation whose COUNT is relaxed + (`AnyTimes()`, `MinTimes(1)`, `Maybe()`, "called at least once") leaves the ARGUMENT matcher as + the only thing asserted; pair it with `gomock.Any()` / `mock.Anything` / `mock.ANY` / + `expect.anything()` / an ignored request body and the expectation proves nothing — a call with + the wrong ID, wrong quantity, or wrong principal satisfies it. Flag every relaxed-count + expectation whose matcher is permissive; the fix pins the load-bearing fields in the matcher + (or exact count + captured args). ## 3. Clarity diff --git a/assets/skills/security-review/SKILL.md b/assets/skills/security-review/SKILL.md index 3b8c415..cbbf577 100644 --- a/assets/skills/security-review/SKILL.md +++ b/assets/skills/security-review/SKILL.md @@ -56,6 +56,16 @@ If no posture is given, assume `standard` and say so in the report. 8. **Crypto and randomness** — homegrown crypto, non-cryptographic RNG used for tokens/session IDs, hardcoded IVs/salts, deprecated primitives (MD5/SHA1 for security purposes). 9. **Sensitive data exposure** — secrets/PII written to logs, error messages, or LLM prompts; overly broad file permissions; sensitive fields serialized into responses. 10. **Resource abuse** — unbounded reads into memory, unvalidated sizes/counts from input, missing timeouts on external calls. +11. **Error-text passthrough on a NEW surface** — a new mount, gateway, adapter, or endpoint that + writes an upstream/middleware error's text (`err.Error()`, connect/gRPC `Message()`, a wrapped + library message) into its own response body. `fs_grep` the new surface's response writers for + interpolated error strings. Default expectation: fixed strings per error class; passthrough + needs POSITIVE justification (the upstream text is a documented, stable, caller-facing + contract). "The same principal already sees this text on another surface" is NOT a clearance + — upstream strings are not a stable contract, grow internal detail over time (verifier + internals, allow-list names, hostnames), and the new surface may acquire callers with + different trust. Medium by default; High when the passed-through source is an authorizer, + token verifier, or anything that names internal identifiers. ## Ground-truth verification (verify, don't pattern-match)