Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions keystore/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ func (m *Module) Init(deps *app.ModuleDeps) error {
m.store = s

for _, ss := range s.belowRecommended() {
// "name", not "key": the logger's SensitiveDataFilter masks any field
// whose name contains "key" (see logger.DefaultFilterConfig).
// "name", not "key": what is logged is a keystore entry's logical name,
// and "name" says so. The filter no longer masks a bare "key" (ADR-072),
// so this is no longer a workaround for one — it is just the right word.
m.logger.Warn().
Str("name", ss.name).
Int("bytes", ss.n).
Expand Down
24 changes: 21 additions & 3 deletions logger/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,27 @@ func DefaultFilterConfig() *FilterConfig {
return &FilterConfig{
SensitiveFields: []string{
sensitiveFieldPassword, "passwd", "pwd",
// "key" masks every field whose name contains it — "keys", "tenant_key",
// "cache_key" — so name a logical identifier "name" or "id", not "key".
sensitiveFieldSecret, "key", sensitiveFieldAPIKey, "apikey",
// Key material is named needle by needle, in both spellings: matching is
// case-insensitive SUBSTRING, so "api_key" does not contain "apikey" and
// neither contains the other. A bare "key" needle used to stand in for
// all of them and masked every field merely containing the word —
// "keys", "tenant_key", "cache_key", and the framework's own "key"
// identifier — with no way to unmask one short of replacing this whole
// list (#1037). "secret_key" and "secretkey" need no entry of their own;
// the "secret" needle already covers both. The hyphenated spellings are
// here because httpclient logs whole http.Header maps through this
// filter under LogPayloads, and a header is spelled "X-Api-Key", which
// no underscore needle matches. A bare "-key" would cover every such
// header and also mask "Idempotency-Key", an identifier consumers send
// on every payment POST, so the same rule applies as above: name the
// shape, not the word. A
// spelling this list does not name — "license_key", "hmac_key",
// "Ocp-Apim-Subscription-Key" — logs in clear until a consumer adds it
// via log.sensitivefields.
sensitiveFieldSecret, sensitiveFieldAPIKey, "apikey", "api-key",
"private_key", "privatekey", "private-key",
"signing_key", "signingkey", "signing-key",
"encryption_key", "encryptionkey", "encryption-key",
sensitiveFieldToken, "access_token", "refresh_token",
"auth", "authorization",
"credential", "credentials",
Expand Down
91 changes: 88 additions & 3 deletions logger/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,9 +940,11 @@ func isSensitiveFieldLinearReference(needles []string, fieldName string) bool {
}

// alphabetSweep enumerates every string of length 0-3 over the given
// alphabet. Chosen so it spells out all five 3-byte default needles ("otp",
// "cvv", "cvc", "key", "pwd") and their one-byte-off neighbors, plus a
// separator and a byte that begins no needle.
// alphabet. Chosen so it spells out all four 3-byte default needles ("otp",
// "cvv", "cvc", "pwd") and their one-byte-off neighbors, plus a separator and a
// byte that begins no needle. It still spells "key", which is no longer a needle
// (#1037) — the corpus derives from the live list, so the extra coverage costs
// nothing and pins that "key" stays absent.
func alphabetSweep(alphabet []byte) []string {
n := len(alphabet)
out := make([]string, 0, 1+n+n*n+n*n*n)
Expand Down Expand Up @@ -1077,3 +1079,86 @@ func TestIsSensitiveFieldDifferentialAgainstLinearReference(t *testing.T) {
})
}
}

// TestDefaultFilterMasksSecretKeyShapesButNotIdentifiers pins both halves of
// dropping the bare "key" needle (#1037). Matching is case-insensitive substring,
// so "key" masked every field whose name merely contained it — "keys",
// "tenant_key", the framework's own "key" identifier — and no consumer could
// unmask one short of replacing the whole default list. The secret-bearing
// spellings it incidentally caught are now named explicitly instead.
func TestDefaultFilterMasksSecretKeyShapesButNotIdentifiers(t *testing.T) {
filter := NewSensitiveDataFilter(DefaultFilterConfig())

tests := []struct {
name string
fieldName string
wantMasked bool
}{
// Explicit needles, in the two spellings the substring matcher treats as
// unrelated: an underscore-separated name does not contain the
// concatenated needle, nor the reverse.
{name: "api_key", fieldName: "api_key", wantMasked: true},
{name: "apikey_concatenated", fieldName: "apikey", wantMasked: true},
{name: "apikey_camel", fieldName: "apiKey", wantMasked: true},
{name: "private_key", fieldName: "private_key", wantMasked: true},
{name: "privatekey_concatenated", fieldName: "privatekey", wantMasked: true},
{name: "privatekey_camel", fieldName: "privateKey", wantMasked: true},
{name: "signing_key", fieldName: "signing_key", wantMasked: true},
{name: "signingkey_concatenated", fieldName: "signingkey", wantMasked: true},
{name: "encryption_key", fieldName: "encryption_key", wantMasked: true},
{name: "encryptionkey_concatenated", fieldName: "encryptionkey", wantMasked: true},
{name: "uppercase_variant", fieldName: "PRIVATE_KEY", wantMasked: true},

// Hyphenated spellings, which no underscore needle matches. httpclient
// logs whole http.Header maps through this filter under LogPayloads, and
// a header is spelled this way.
{name: "http_header_api_key", fieldName: "X-Api-Key", wantMasked: true},
{name: "http_header_api_key_upper", fieldName: "X-API-KEY", wantMasked: true},
{name: "hyphenated_private_key", fieldName: "private-key", wantMasked: true},
{name: "hyphenated_signing_key", fieldName: "signing-key", wantMasked: true},
{name: "hyphenated_encryption_key", fieldName: "encryption-key", wantMasked: true},

// The hyphen needles name shapes, not the bare word: an identifier
// spelled with hyphens stays in clear, exactly as its underscore twin does.
{name: "idempotency_key_header", fieldName: "Idempotency-Key", wantMasked: false},
{name: "hyphenated_routing_key", fieldName: "routing-key", wantMasked: false},
{name: "hyphenated_partition_key", fieldName: "partition-key", wantMasked: false},
{name: "embedded_in_a_longer_name", fieldName: "tenant_private_key_pem", wantMasked: true},

// Carried by the "secret" needle rather than a key-specific one, which is
// why neither spelling is listed twice.
{name: "secret_key", fieldName: "secret_key", wantMasked: true},
{name: "secretkey_concatenated", fieldName: "secretkey", wantMasked: true},

// Identifiers. Every one of these was masked before.
{name: "bare_key", fieldName: "key", wantMasked: false},
{name: "plural_keys", fieldName: "keys", wantMasked: false},
{name: "tenant_key", fieldName: "tenant_key", wantMasked: false},
{name: "cache_key", fieldName: "cache_key", wantMasked: false},
{name: "routing_key", fieldName: "routing_key", wantMasked: false},
{name: "uppercase_identifier", fieldName: "KEY", wantMasked: false},

// Untouched by this change, asserted so a needle list edit cannot quietly
// drop one on its way past.
{name: "password", fieldName: "password", wantMasked: true},
{name: "token", fieldName: "token", wantMasked: true},
{name: "authorization", fieldName: "authorization", wantMasked: true},
{name: "cvv", fieldName: "cvv", wantMasked: true},
{name: "plain_name", fieldName: "name", wantMasked: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := filter.isSensitiveField(tt.fieldName); got != tt.wantMasked {
t.Errorf("isSensitiveField(%q) = %v, want %v", tt.fieldName, got, tt.wantMasked)
}

// The value path is what a caller actually sees, so assert there too
// rather than trusting the predicate alone.
gotValue := filter.FilterString(tt.fieldName, "v")
if (gotValue == DefaultMaskValue) != tt.wantMasked {
t.Errorf("FilterString(%q, \"v\") = %q, want masked=%v", tt.fieldName, gotValue, tt.wantMasked)
}
})
}
}
108 changes: 108 additions & 0 deletions wiki/adr_072_default_log_filter_names_key_material_explicitly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# ADR-072: The default log filter names key material explicitly, not by a bare "key"

- **Status**: Accepted
- **Date**: 2026-08-20
- **Related**: [ADR-070](adr_070_inbound_trace_identifier_validation.md) (the other seam where a value's shape decides whether it survives to a log line) · `wiki/observability.md` (the consumer-facing list and the two extension seams)

## Context

`logger.DefaultFilterConfig` masks a value when its **field name** matches a
needle, case-insensitively, by substring. The list carried a bare `key`.

Substring matching makes that needle far wider than key material. Every field
whose name merely contains the word is masked: `keys`, `tenant_key`,
`cache_key`, `routing_key`, and the plain `key` the framework itself logs.
Fifteen of the framework's own log sites log the bare field name `key` — nine in the
app factory resolver, four in the messaging manager, one each in the database
manager and the server handler. Fourteen log a tenant or resource identifier
under it; the fifteenth logs the NAME of a reserved envelope-meta key a handler
tried to overwrite. Every one renders `***`, and so do the eight `routing_key`
sites in the AMQP client and registry, and `dropped_meta_keys`.

The loss is one-way and unrecoverable at the consumer's end. The YAML seam
(`log.sensitivefields`) only *adds* needles; there is no removal. A consumer who
wants `tenant_key` in their logs has to abandon the default list wholesale
through `app.Options.LoggerFilterConfig` and re-supply every entry, which is
precisely the mistake that seam already warns about. So the bare needle taxes
observability for everyone and offers no dial.

What it did buy is real, though: it incidentally covered `private_key`,
`signing_key`, `encryption_key` and their concatenated spellings. Removing it
without replacement would put actual key material in the clear.

## Decision

Name the key-material shapes explicitly, then drop the bare needle.

Added: `private_key`, `privatekey`, `private-key`, `signing_key`, `signingkey`,
`signing-key`, `encryption_key`, `encryptionkey`, `encryption-key`, `api-key`.
Already present and kept: `api_key`, `apikey`.

Three spellings of each, because the matcher gives no relationship between them:
`api_key` does not contain `apikey`, and neither contains `api-key`. One needle
covers only the exact byte sequence, wherever it appears.

The hyphenated ones are not hypothetical tidiness. `httpclient` logs whole
`http.Header` maps through this filter under `LogPayloads`, and the header is
spelled `X-Api-Key` — which the bare `key` masked and no underscore needle does.
A single `-key` needle would cover every such header, and would also mask
`Idempotency-Key` — an identifier consumers send on every payment POST, and
exactly the over-masking this change exists to end. Naming the shape rather than
the word is the same rule this ADR applies everywhere else.

`secret_key` and `secretkey` are deliberately NOT added. Both contain `secret`,
which is already a needle, so entries would be dead weight that reads as
coverage. That asymmetry is worth stating rather than leaving for a reader to
re-derive from the matcher.

Identifiers — `key`, `keys`, `tenant_key`, `cache_key`, `routing_key` — now log
in clear, and the framework's fifteen sites emit their identifiers without a
rename.

## Alternatives considered

**Rename the framework's log fields `key` → `name`/`id`.** Rejected during
triage. It fixes the framework's own fifteen lines and leaves every consumer
field — `tenant_key`, `cache_key` — masked, so the general problem survives and
the framework's log field names become a compatibility surface.

**Exempt by suffix or prefix: mask `*_key` but not `key`/`keys`.** Fragile in
both directions. `license_key` is a secret and `routing_key` is not, and no
affix rule separates them; it also introduces a second matching mode into a
filter whose one rule today is "substring", which is the property that makes the
list auditable.

**Keep the needle and add a removal seam to `log.sensitivefields`.** A larger
API — subtraction against a list the framework may change under you, with the
failure mode of silently un-masking something a later release adds. The list is
the contract; editing what is *in* it is the honest change.

**Do nothing; tell consumers to name identifiers `id`.** This is what the code
comment said before. It does not survive contact with an existing service, and
it puts the framework in the position of dictating field names to avoid its own
default.

## Consequences

**Positive.** Fifteen framework log sites regain their identifiers with no
rename. Consumer fields named `*_key` that are identifiers stop being masked.
The list now says what it covers instead of relying on a word that happens to
appear inside key material.

**Negative — this un-masks.** A consumer relying on the bare needle to mask a
field the new list does not name — `license_key`, `hmac_key`, `master_key`,
`session_key`, or a vendor header such as `Ocp-Apim-Subscription-Key` — starts
logging that value in clear on upgrade, with no error and no warning.

One shape in that class deserves naming on its own, because it does not read as
a secret at all: `keys` is the JWKS container. The bare needle stopped the filter's walk at that
field; now it recurses, and a JWK's `d` — the RSA private exponent — matches no
needle. That reaches a log only through `httpclient`'s `LogPayloads`, which is
off by default and documented dev-only, but a service that turns it on while
fetching a PRIVATE key set logs the private material. Add `keys` to
`log.sensitivefields` if that is your shape. That is the whole risk of this change and it is why it
ships with a migration atom rather than as a quiet default tweak; the remedy is
one line of `log.sensitivefields`. Documented as `[C60.13]`.

**Neutral.** Nothing about the matcher changes: still case-insensitive, still
substring, still applied at the same seam. Only the list moved.
22 changes: 22 additions & 0 deletions wiki/architecture_decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,28 @@ v0.59 `SlowJob` godoc suggested one, and that disable path was never reachable),
`*config.Config` handed straight to `Module.Init`/`NewModuleRegistry` is never normalized — see
`[C60.12]`.

### [ADR-072: The Default Log Filter Names Key Material Explicitly, Not by a Bare "key"](adr_072_default_log_filter_names_key_material_explicitly.md)

**Date:** 2026-08-20 | **Status:** Accepted

`logger.DefaultFilterConfig` matches field names by case-insensitive SUBSTRING, so its bare
`key` needle masked every field merely containing the word — `keys`, `tenant_key`, `cache_key`,
and the plain `key` the framework logs at fifteen of its own sites, all of them tenant or
resource identifiers. The YAML seam only adds needles, so no consumer could unmask one without
abandoning the whole default list. Key material is now named needle by needle instead — all three
of the `api_key`, `apikey` and `api-key` spellings of each, since the matcher relates them not at
all, and the hyphenated ones because `httpclient` logs whole `http.Header` maps through this
filter under `LogPayloads` — and the bare needle is gone.

**Key Benefits:** Identifiers log in clear without renaming a single field, and the list states
its coverage instead of leaning on a word that happens to appear inside secrets.
**Watch:** this UN-MASKS. A field the new list does not name — `license_key`, `hmac_key`,
`Ocp-Apim-Subscription-Key`, or the JWKS container `keys` — starts logging in clear on upgrade,
silently; the remedy is one `log.sensitivefields` entry. `secret_key` needs no needle: `secret`
already covers it. See [migrations.md](migrations.md) `[C60.13]`.

---

### [ADR-071: Upsert Column Sets Name Each Column Once, in a Form the Vendor Can Name](adr_071_upsert_column_sets_name_each_column_once.md)

**Date:** 2026-08-20 | **Status:** Accepted
Expand Down
Loading