feat: applied the whetstone's adversary and security-review change suggestions
This commit is contained in:
3 files changed
+30
-1
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in new issue
Block a user