.agents/skills/security-review/references/secret-read-paths.md
Feature read scopes (automations:read, integrations:read, and siblings) are
held by every project role, VIEWER included. Anything a read path returns is
therefore readable by the least-privileged member of the project and ships to
the browser.
Config blobs stored as JSON columns mix display fields with credentials in the
same object: a webhook config carries secretKey next to displaySecretKey, a
repository-dispatch config carries githubToken next to displayGitHubToken,
integration configs carry encrypted request headers next to their display
values. Encryption at rest does not make such a field safe to return. The
ciphertext still widens the blast radius of an ENCRYPTION_KEY compromise, and
the repo's own standard — visible in the sibling fields that are stripped — is
that these never reach a client.
The recurring shape of the bug: sanitization is written per type, wired into the write path, and the read path keeps a catch-all fallback for "other types". The fallback silently ships the next type's credentials, and the type assertion on it hides the omission from the compiler.
convertToSafeWebhookConfig / convertToSafeGitHubDispatchConfig
— they name every field that may leave the server, so a newly added field is
absent from responses until someone adds it deliberately.Safe* schemas defined as FullSchema.omit({ <secret fields> }), so the safe
type and the field allowlist (Object.keys(SafeXSchema.shape)) stay derived
from one declaration.automation-repository.ts:
getActionById returns the sanitized config for anything client-facing,
getActionByIdWithSecrets returns the raw row and is reserved for execution
paths (worker delivery, config helpers).omit block in
db.ts excludes secret-bearing
columns from every Prisma result by default; delivery paths opt back in with
an explicit select. Prefer this over hand-stripping when the secret is its
own column rather than a JSON field.Sanitize in the shared repository or domain converter, not in the route. One converter must serve every read route and the create/update responses. A router-level sanitizer only covers the surfaces its author remembered.
Dispatch exhaustively over the type union. Use a switch whose default
assigns to never:
default: {
const unhandledActionType: never = actionType;
throw new InternalServerError(`unhandled type ${unhandledActionType}`);
}
A new union member is then a compile error until it has a sanitizer. Never write a fallback that returns the stored value for "other or future types".
Build the safe object by allowlist, not by deleting known secrets. A deny-list is only as current as the last person who added a field.
Fail closed on values that do not parse. Project the stored object onto the safe schema's keys instead of passing it through — legacy and hand-edited rows are exactly the ones that skip a parse-gated sanitizer. Prefer projection over throwing: reads stay available while secrets still cannot survive.
Add a negative test on the read path, as the least-privileged role.
Assert expect(config).not.toHaveProperty("<secret>") for a VIEWER caller
against both the list route and the single-item route. A clean
create/update response proves nothing about the read path — that asymmetry
is how this class of bug survives review.
convertActionToDomain
— exhaustive switch, per-type allowlist sanitizer, allowlist projection as
the parse-failure fallback; the single converter behind the automations read
routes and the create/update responses in
router.ts.describe("automations read path secret redaction") in
automations-trpc.servertest.ts
— VIEWER-role read-path negative tests, including a config that fails to
parse.config: row.config as SafeActionConfig. The name claims the invariant while the cast suppresses
the only check that would enforce it. Flag every as Safe*, as Public*,
as Redacted*. If the type carries a security invariant, consider branding
it so that only the converter can produce a value of that type and a bare
cast stops compiling.Safe*
config into prisma.<model>.update({ data: { config } }) persists the
stripped shape and silently drops the fields the sanitizer removed
(encrypted headers, stored secrets). Re-read the raw row for writes.