--- title: "Concerns" description: "Every card's rule on one page. Pick a concern, such as performance or security, to scope a review." canonical: "https://goodcodereview.com/concerns" updated: "2026-10-25" --- # Concerns Stable cards grouped by concern. A card appears under each concern it lists. Report each check as `PASS`, `FAIL`, or `N/A` with `file:line` evidence. A `FAIL` carries the card's severity. Any `block` `FAIL` means request changes. A `should-fix` or `discuss` `FAIL` goes in the review but does not block. ## correctness - [`code-consistency-drift`](https://goodcodereview.com/codebase.md#code-consistency-drift) [discuss]: For each recurring concern (errors, config, HTTP clients, logging), make one pattern the standard instead of leaving several as equal peers. - [`fn-independent-work-concurrent`](https://goodcodereview.com/function.md#fn-independent-work-concurrent) [should-fix]: Start independent I/O together and wait for all of it; run calls in sequence only when one needs the result of another. - [`fn-no-blocking-call-in-async-code`](https://goodcodereview.com/function.md#fn-no-blocking-call-in-async-code) [block]: Never make a blocking I/O call or run long CPU work directly in async code; use the async API or hand the work to a thread or worker. - [`mod-single-writer-per-state`](https://goodcodereview.com/module.md#mod-single-writer-per-state) [should-fix]: Let only one writer change a piece of state at a time, through a single owner, a queue that serialises the writes, or compare-and-swap, never by convention. - [`svc-api-conditional-writes`](https://goodcodereview.com/service.md#svc-api-conditional-writes) [should-fix]: Return an `ETag` on reads and honour `If-Match` on writes, answering 412 when the entity tag is stale. - [`svc-bounded-background-tasks`](https://goodcodereview.com/service.md#svc-bounded-background-tasks) [block]: Cap how many background tasks run at once, and never spawn one per input with no limit. - [`svc-idempotency-key`](https://goodcodereview.com/service.md#svc-idempotency-key) [block]: Make every retryable write idempotent: the caller sends a unique key, and a retry with the same key never applies the write twice. - [`sys-lock-fencing`](https://goodcodereview.com/system.md#sys-lock-fencing) [block]: Check a correctness lock on the resource side (fencing token or compare-and-set), never by lock possession alone. - [`sys-secrets-and-errors`](https://goodcodereview.com/system.md#sys-secrets-and-errors) [block]: Fail closed: a validation or auth error must deny, never continue, and an error response must not carry internals such as stack traces, SQL or tokens. - [`chg-match-codebase-pattern`](https://goodcodereview.com/change.md#chg-match-codebase-pattern) [should-fix]: Follow the established pattern nearby. ## security - [`code-dependency-licence-sbom`](https://goodcodereview.com/codebase.md#code-dependency-licence-sbom) [should-fix]: Generate an SBOM, and scan dependencies for known vulnerabilities on every change or on a schedule. - [`code-dependency-pinning`](https://goodcodereview.com/codebase.md#code-dependency-pinning) [block]: Commit a lockfile with hashes and install from it in CI. - [`code-secret-scan`](https://goodcodereview.com/codebase.md#code-secret-scan) [block]: Scan for committed credentials, and treat any hit as leaked: revoke and replace it first. - [`code-workflow-supply-chain`](https://goodcodereview.com/codebase.md#code-workflow-supply-chain) [block]: Pin every CI action to a full commit SHA. - [`fn-confine-file-path`](https://goodcodereview.com/function.md#fn-confine-file-path) [block]: Resolve the final path and check that it stays inside the intended base directory before using it. - [`fn-csprng-for-secrets`](https://goodcodereview.com/function.md#fn-csprng-for-secrets) [block]: Draw every security-relevant random value from the platform CSPRNG. - [`fn-jwt-verify-pinned`](https://goodcodereview.com/function.md#fn-jwt-verify-pinned) [block]: Verify the signature against a fixed list of accepted algorithms, and check `iss` and `aud`, before reading any claim. - [`fn-no-mass-assignment`](https://goodcodereview.com/function.md#fn-no-mass-assignment) [block]: Bind request data to a DTO or an explicit field allowlist, never straight onto the model. - [`fn-no-native-deserialise-untrusted`](https://goodcodereview.com/function.md#fn-no-native-deserialise-untrusted) [block]: Parse untrusted input with a data-only format, never with a native object deserialiser. - [`fn-no-raw-html-sink`](https://goodcodereview.com/function.md#fn-no-raw-html-sink) [block]: Write untrusted text through an encoding sink (`textContent`, auto-escaped template), never through a raw HTML sink. - [`fn-no-shell-with-input`](https://goodcodereview.com/function.md#fn-no-shell-with-input) [block]: Start processes with the program and each argument as separate list items, and never through a shell string. - [`fn-object-level-authz`](https://goodcodereview.com/function.md#fn-object-level-authz) [block]: Check that the caller may act on the specific record before returning or changing it. - [`fn-password-slow-hash`](https://goodcodereview.com/function.md#fn-password-slow-hash) [block]: Hash passwords with Argon2id (or scrypt, bcrypt, PBKDF2) and a per-password salt from the library. - [`fn-response-fields-explicit`](https://goodcodereview.com/function.md#fn-response-fields-explicit) [should-fix]: Return a response type that lists each field, never the stored model or a generic dump of it. - [`fn-tls-verify-on`](https://goodcodereview.com/function.md#fn-tls-verify-on) [block]: Leave certificate and hostname verification on in every environment that ships. - [`mod-vetted-crypto-only`](https://goodcodereview.com/module.md#mod-vetted-crypto-only) [block]: Encrypt with an authenticated mode such as AES-GCM, and never reuse a nonce under one key. - [`svc-auth-throttle-generic-errors`](https://goodcodereview.com/service.md#svc-auth-throttle-generic-errors) [block]: Limit failed attempts per account, and return the same message and status code whether or not the account exists. - [`svc-authz-central-deny-default`](https://goodcodereview.com/service.md#svc-authz-central-deny-default) [block]: Route every request through one authorization layer that denies when no rule allows it. - [`svc-bound-client-controlled-sizes`](https://goodcodereview.com/service.md#svc-bound-client-controlled-sizes) [should-fix]: Cap every client-controlled size and count on the server: body bytes, string length, array items, page size and batch size. - [`svc-cors-allowlist`](https://goodcodereview.com/service.md#svc-cors-allowlist) [should-fix]: Allow cross-origin access only to a fixed list of named origins, and never echo the request `Origin` header back unchecked. - [`svc-csrf-state-changing`](https://goodcodereview.com/service.md#svc-csrf-state-changing) [block]: Require a CSRF token (or an equivalent header check) on every cookie-authenticated request that changes state, and never change state on GET. - [`svc-rate-limit-quota`](https://goodcodereview.com/service.md#svc-rate-limit-quota) [should-fix]: Enforce a per-tenant quota on shared APIs and queues. - [`svc-session-lifecycle`](https://goodcodereview.com/service.md#svc-session-lifecycle) [block]: Issue a new session ID at login, and set `Secure`, `HttpOnly` and `SameSite` on the session cookie. - [`svc-sqli-authn`](https://goodcodereview.com/service.md#svc-sqli-authn) [block]: Pass untrusted data to queries as bound parameters, never by string concatenation or formatting. - [`svc-upload-validate-store`](https://goodcodereview.com/service.md#svc-upload-validate-store) [block]: Allowlist the file type, cap the size, and store each upload under a generated name where the server will not execute it. - [`sys-agent-tool-least-privilege`](https://goodcodereview.com/system.md#sys-agent-tool-least-privilege) [block]: Give the agent only the narrow tools and the minimal credentials its task needs. - [`sys-pii-minimise-retention`](https://goodcodereview.com/system.md#sys-pii-minimise-retention) [should-fix]: Give every store of personal or sensitive data a retention period, and delete or anonymise the data when it ends. - [`sys-prompt-injection`](https://goodcodereview.com/system.md#sys-prompt-injection) [block]: Validate model output deterministically before code acts on it. - [`sys-secrets-and-errors`](https://goodcodereview.com/system.md#sys-secrets-and-errors) [block]: Fail closed: a validation or auth error must deny, never continue, and an error response must not carry internals such as stack traces, SQL or tokens. - [`sys-ssrf`](https://goodcodereview.com/system.md#sys-ssrf) [block]: Fetch a user-supplied URL only when its host matches a server-side allowlist. ## performance - [`code-perf-budget-in-ci`](https://goodcodereview.com/codebase.md#code-perf-budget-in-ci) [should-fix]: Write the performance budget down as a CI check that fails the build when it is exceeded. - [`fn-fetch-only-needed-data`](https://goodcodereview.com/function.md#fn-fetch-only-needed-data) [should-fix]: Filter, count, and aggregate in the query, and select only the columns the code uses. - [`fn-independent-work-concurrent`](https://goodcodereview.com/function.md#fn-independent-work-concurrent) [should-fix]: Start independent I/O together and wait for all of it; run calls in sequence only when one needs the result of another. - [`fn-join-dont-concat-in-loop`](https://goodcodereview.com/function.md#fn-join-dont-concat-in-loop) [should-fix]: Build a string from a loop with a builder or a join, never with repeated `+=`. - [`fn-no-blocking-call-in-async-code`](https://goodcodereview.com/function.md#fn-no-blocking-call-in-async-code) [block]: Never make a blocking I/O call or run long CPU work directly in async code; use the async API or hand the work to a thread or worker. - [`fn-regex-compiled-once`](https://goodcodereview.com/function.md#fn-regex-compiled-once) [should-fix]: Compile a regular expression once, at module or startup level, and reuse it. - [`inc-perf-fix-has-profile-evidence`](https://goodcodereview.com/incident.md#inc-perf-fix-has-profile-evidence) [should-fix]: Tie the performance fix to evidence that names the bottleneck: a profile, a trace, or a USE check. - [`mod-pooled-clients-built-once`](https://goodcodereview.com/module.md#mod-pooled-clients-built-once) [should-fix]: Build pooled clients once at startup and reuse them. - [`prod-latency-slo-percentile`](https://goodcodereview.com/production.md#prod-latency-slo-percentile) [should-fix]: Define latency SLOs and alerts on high percentiles, never on the mean. - [`prod-load-test-before-launch`](https://goodcodereview.com/production.md#prod-load-test-before-launch) [should-fix]: Load test before launch, past the expected peak, with pass and fail thresholds written down. - [`prod-profile-on-demand`](https://goodcodereview.com/production.md#prod-profile-on-demand) [discuss]: Make a CPU and memory profile of a production instance obtainable, through a continuous profiler or a private profile endpoint. - [`svc-list-endpoint-paginates`](https://goodcodereview.com/service.md#svc-list-endpoint-paginates) [should-fix]: Paginate every list endpoint, with a default page size and a server-side maximum. - [`svc-timeouts`](https://goodcodereview.com/service.md#svc-timeouts) [block]: Set an explicit timeout on every remote call. - [`sys-cache-bounds-invalidation`](https://goodcodereview.com/system.md#sys-cache-bounds-invalidation) [should-fix]: Bound every cache by size and give its entries an expiry. - [`sys-fanout-leg-deadlines`](https://goodcodereview.com/system.md#sys-fanout-leg-deadlines) [should-fix]: Give every leg of a fan-out its own deadline, and decide in code what happens when a leg misses it: a partial result, a hedged request, or an error. - [`sys-n-plus-one-pooling`](https://goodcodereview.com/system.md#sys-n-plus-one-pooling) [should-fix]: Fetch related rows in one query or batch, never one query per row. - [`chg-perf-claim-evidence`](https://goodcodereview.com/change.md#chg-perf-claim-evidence) [should-fix]: Back a performance claim with measured before-and-after numbers. ## reliability - [`fn-rerun-safe-after-crash`](https://goodcodereview.com/function.md#fn-rerun-safe-after-crash) [block]: Make a state-changing operation give the same result on a second run and after a crash at any step, by deriving each step from current state and making leftovers recognisable as stale. - [`inc-rollback-path-tested`](https://goodcodereview.com/incident.md#inc-rollback-path-tested) [should-fix]: Exercise each mitigation that a runbook names (rollback, failover, flag off) on a regular schedule. - [`prod-canary-staged-rollout`](https://goodcodereview.com/production.md#prod-canary-staged-rollout) [block]: Roll out in stages behind a canary, and abort by rule, not by judgment. - [`prod-kill-switch-fallback`](https://goodcodereview.com/production.md#prod-kill-switch-fallback) [should-fix]: Make a risky feature switchable off without a deploy, and exercise every fallback path in test or in production on a schedule. - [`prod-load-test-before-launch`](https://goodcodereview.com/production.md#prod-load-test-before-launch) [should-fix]: Load test before launch, past the expected peak, with pass and fail thresholds written down. - [`svc-bounded-background-tasks`](https://goodcodereview.com/service.md#svc-bounded-background-tasks) [block]: Cap how many background tasks run at once, and never spawn one per input with no limit. - [`svc-idempotency-key`](https://goodcodereview.com/service.md#svc-idempotency-key) [block]: Make every retryable write idempotent: the caller sends a unique key, and a retry with the same key never applies the write twice. - [`svc-rate-limit-quota`](https://goodcodereview.com/service.md#svc-rate-limit-quota) [should-fix]: Enforce a per-tenant quota on shared APIs and queues. - [`svc-retry-policy`](https://goodcodereview.com/service.md#svc-retry-policy) [block]: Retry only transient errors, with capped attempts and jittered exponential backoff. - [`svc-timeouts`](https://goodcodereview.com/service.md#svc-timeouts) [block]: Set an explicit timeout on every remote call. - [`sys-fanout-leg-deadlines`](https://goodcodereview.com/system.md#sys-fanout-leg-deadlines) [should-fix]: Give every leg of a fan-out its own deadline, and decide in code what happens when a leg misses it: a partial result, a hedged request, or an error. - [`sys-load-shedding-bounded-queues`](https://goodcodereview.com/system.md#sys-load-shedding-bounded-queues) [block]: Bound every queue and reject excess work cheaply. - [`sys-lock-fencing`](https://goodcodereview.com/system.md#sys-lock-fencing) [block]: Check a correctness lock on the resource side (fencing token or compare-and-set), never by lock possession alone. - [`sys-migration-expand-contract`](https://goodcodereview.com/system.md#sys-migration-expand-contract) [block]: Run data backfills in batches, in a migration separate from the schema change. ## compatibility - [`fn-response-fields-explicit`](https://goodcodereview.com/function.md#fn-response-fields-explicit) [should-fix]: Return a response type that lists each field, never the stored model or a generic dump of it. - [`mod-derive-dont-duplicate`](https://goodcodereview.com/module.md#mod-derive-dont-duplicate) [should-fix]: Derive a type, list or enum from the schema or list that owns it, or add a build check that fails when the two disagree. - [`svc-api-deprecation-sunset`](https://goodcodereview.com/service.md#svc-api-deprecation-sunset) [should-fix]: Before removing an endpoint, mark it `deprecated` in the spec and send `Deprecation` and `Sunset` headers, with the `Sunset` date no earlier than the `Deprecation` date. - [`svc-api-error-code-stable`](https://goodcodereview.com/service.md#svc-api-error-code-stable) [should-fix]: Give each error a stable machine-readable code that callers branch on, and never make them parse the message text. - [`svc-api-versioning-policy`](https://goodcodereview.com/service.md#svc-api-versioning-policy) [should-fix]: Write down one versioning scheme for the API, and ship no breaking change inside a version it has published. - [`sys-migration-expand-contract`](https://goodcodereview.com/system.md#sys-migration-expand-contract) [block]: Run data backfills in batches, in a migration separate from the schema change. - [`chg-breaking-change-taxonomy`](https://goodcodereview.com/change.md#chg-breaking-change-taxonomy) [block]: Treat any observable difference in signature, return shape, error type, ordering, timing, defaults, or side effects as breaking until a consumer check says otherwise. - [`chg-expand-contract`](https://goodcodereview.com/change.md#chg-expand-contract) [block]: Evolve shared contracts in steps: add the new form beside the old (expand), migrate every consumer, then remove the old form (contract) in a later change. ## testing - [`inc-perf-fix-has-profile-evidence`](https://goodcodereview.com/incident.md#inc-perf-fix-has-profile-evidence) [should-fix]: Tie the performance fix to evidence that names the bottleneck: a profile, a trace, or a USE check. - [`chg-perf-claim-evidence`](https://goodcodereview.com/change.md#chg-perf-claim-evidence) [should-fix]: Back a performance claim with measured before-and-after numbers. - [`chg-refactor-has-behaviour-pin`](https://goodcodereview.com/change.md#chg-refactor-has-behaviour-pin) [should-fix]: Pin current behaviour with a characterization test, snapshot or equivalence check before the refactor moves any structure. - [`chg-tests-ship-with-change`](https://goodcodereview.com/change.md#chg-tests-ship-with-change) [block]: Ship new or changed behaviour in the same change as a test that fails if the behaviour breaks; a code-only change needs a stated reason. - [`chg-verify-before-claim`](https://goodcodereview.com/change.md#chg-verify-before-claim) [block]: Back every "works" or "bug" claim with something run: a repro, a test or command output. ## maintainability - [`code-adr`](https://goodcodereview.com/codebase.md#code-adr) [discuss]: Keep a short written record for each significant design choice (datastore, framework, deviation from a pattern) that states the context, the decision, and its consequences. - [`code-consistency-drift`](https://goodcodereview.com/codebase.md#code-consistency-drift) [discuss]: For each recurring concern (errors, config, HTTP clients, logging), make one pattern the standard instead of leaving several as equal peers. - [`code-dead-code`](https://goodcodereview.com/codebase.md#code-dead-code) [should-fix]: Remove unreferenced functions, commented-out code, expired flags, and orphaned files; history keeps them. - [`code-rule-has-enforcer`](https://goodcodereview.com/codebase.md#code-rule-has-enforcer) [should-fix]: Back each rule that a type, lint or CI check could enforce with that check, not with a line in an instruction file or a repeated review comment. - [`fn-comments-explain-why`](https://goodcodereview.com/function.md#fn-comments-explain-why) [should-fix]: Write comments that give a reason, a constraint, or a trade-off; rename before commenting, and never restate what the next line does. - [`mod-derive-dont-duplicate`](https://goodcodereview.com/module.md#mod-derive-dont-duplicate) [should-fix]: Derive a type, list or enum from the schema or list that owns it, or add a build check that fails when the two disagree. - [`mod-yagni`](https://goodcodereview.com/module.md#mod-yagni) [should-fix]: Add an abstraction only when the logic is reused, non-trivial, or clutters the main flow; do not build for a need that does not exist yet. - [`chg-match-codebase-pattern`](https://goodcodereview.com/change.md#chg-match-codebase-pattern) [should-fix]: Follow the established pattern nearby. - [`chg-refactor-has-behaviour-pin`](https://goodcodereview.com/change.md#chg-refactor-has-behaviour-pin) [should-fix]: Pin current behaviour with a characterization test, snapshot or equivalence check before the refactor moves any structure. - [`chg-refactor-separate`](https://goodcodereview.com/change.md#chg-refactor-separate) [should-fix]: Put refactors, renames, and reformatting in their own commit or PR, apart from behaviour changes. ## accessibility - [`fn-aria-role-has-state-and-keys`](https://goodcodereview.com/function.md#fn-aria-role-has-state-and-keys) [should-fix]: When a native element cannot do the job, give the custom widget the state attribute and the keys its role promises. - [`fn-colour-not-sole-cue`](https://goodcodereview.com/function.md#fn-colour-not-sole-cue) [should-fix]: Pair every colour that carries meaning with a second cue: text, an icon, a shape, or an underline. - [`fn-dialog-focus-managed`](https://goodcodereview.com/function.md#fn-dialog-focus-managed) [block]: When a modal opens, move focus into it and keep Tab inside it. Make Escape close it. - [`fn-error-says-what-and-how`](https://goodcodereview.com/function.md#fn-error-says-what-and-how) [should-fix]: Show each validation error next to its field, in plain words that name what is wrong and how to fix it. - [`fn-focus-indicator-visible`](https://goodcodereview.com/function.md#fn-focus-indicator-visible) [block]: Keep a visible focus indicator on every focusable element. Never set `outline: none` without a replacement. - [`fn-form-control-has-label`](https://goodcodereview.com/function.md#fn-form-control-has-label) [block]: Tie every form control to a visible `