Redact secrets from logs and downgrade PII log levels by ChristianPavilonis · Pull Request #468 · IABTechLab/trusted-server

ChristianPavilonis · 2026-03-10T01:52:26Z

Summary

Security fix: Secrets (proxy_secret, secret_key, handler passwords) were logged in plaintext at INFO/DEBUG level. Introduced a Redacted<T> wrapper that prints [REDACTED] in Debug/Display while preserving transparent serde and .expose() access.
Privacy fix: PII (client IP, user-agent) and full request/response payloads were logged at INFO/DEBUG. Downgraded PII logs to debug! and payload logs to trace! so they no longer appear in production.
Logger level: Changed production logger from Debug to Info to prevent verbose output in production by default.

Changes

File	Change
`crates/common/src/redacted.rs`	New `Redacted<T>` wrapper type with `[REDACTED]` Debug/Display, transparent serde, `.expose()`, and tests
`crates/common/src/lib.rs`	Added `mod redacted`
`crates/common/src/settings.rs`	Changed `proxy_secret`, `secret_key`, `username`, `password`, `secret_store_id` to `Redacted<String>`; updated validators
`crates/common/build.rs`	Added redacted module `#[path]` for build
`crates/common/src/auth.rs`	Use `.expose()` for credential comparison
`crates/common/src/http_util.rs`	Use `.expose()` for proxy_secret access
`crates/common/src/settings_data.rs`	Use `.expose()` for secret_key comparison
`crates/common/src/request_signing/endpoints.rs`	Use `.expose()` for secret_store_id
`crates/common/src/synthetic.rs`	Downgraded 5 PII log calls (client IP, user-agent, synthetic IDs) from `info!` to `debug!`
`crates/common/src/integrations/prebid.rs`	Downgraded payload logging from `debug!` to `trace!`
`crates/common/src/integrations/aps.rs`	Downgraded payload logging from `debug!` to `trace!`
`crates/common/src/integrations/adserver_mock.rs`	Downgraded payload logging from `debug!` to `trace!`
`crates/fastly/src/main.rs`	Logger level `Debug` → `Info`; settings log `info!` → `debug!`
`AGENTS.md`	Fixed incorrect `tracing` reference to `log`

Closes

Closes #404

Test plan

cargo test --workspace
cargo clippy --all-targets --all-features -- -D warnings
cargo fmt --all -- --check
JS tests: cd crates/js/lib && npx vitest run
JS format: cd crates/js/lib && npm run format
Docs format: cd docs && npm run format
WASM build: cargo build --bin trusted-server-fastly --release --target wasm32-wasip1
Manual testing via fastly compute serve

Checklist

Changes follow CLAUDE.md conventions
No unwrap() in production code — use expect("should ...")
Uses log macros (not println!)
New code has tests
No secrets or credentials committed

aram356

Summary

This PR introduces a Redacted<T> wrapper type for secrets, downgrades PII/payload log levels from info!/debug! to debug!/trace!, and hard-caps the production log level to Info. Good foundational work — two issues need addressing before merge.

Blocking

1. PII/payload leak at WARN level in Prebid error path
crates/common/src/integrations/prebid.rs lines 879-884

log::warn! logs up to 1000 chars of raw response body on non-2xx status. This bypasses the debug/trace downgrades and will emit in production with the current Info max level. If Prebid returns echoed request/debug blobs, this can include user/device data.

Suggestion: only log the status code at warn! and move the body preview to trace!.

2. Insecure-secret sentinel checks are inconsistent
crates/common/src/settings.rs line 200 + crates/common/src/settings_data.rs line 37

See inline comments. validate_secret_key rejects "secret_key" (underscore) while settings_data.rs rejects "secret-key" (hyphen) — two different strings guarding against the same mistake with split behavior.

Non-blocking

See inline comments for details on each:

ValidationError codes (settings.rs:199): new() takes a code, not a human message
Missing regression test (cross-cutting): no test asserting format!("{:?}", Settings) excludes secret literals across all secret-bearing fields — if a future field forgets Redacted<T>, there is no safety net
build.rs #[path] coupling (build.rs:9): fragile inclusion of redacted.rs; needs a guard comment
Test code still logs synthetic ID (synthetic.rs:266): log::info!("Generated synthetic ID: {}", synthetic_id) in test — minor inconsistency with PR goal
PII at debug level (synthetic.rs:99): template input with raw IP/UA downgraded to debug! but not trace! like other payload logs
Hard-capped log level (main.rs:177): follow-up to make max level configurable for incident debugging
Non-constant-time auth comparison (auth.rs:17): out of scope, follow-up for subtle::ConstantTimeEq

CI Status

All 10 checks PASS: cargo fmt, cargo test, vitest, CodeQL, format-docs, format-typescript, Analyze (actions, javascript-typescript x2, rust).

crates/common/src/settings.rs

crates/common/src/settings_data.rs

crates/common/build.rs

crates/common/src/synthetic.rs

crates/fastly/src/main.rs

crates/common/src/auth.rs

crates/common/src/redacted.rs

crates/common/src/integrations/prebid.rs

- Split Prebid warn log so status stays at warn! and body moves to trace! - Consolidate secret-key placeholder checks into validate_secret_key - Use proper ValidationError codes (snake_case identifiers) - Remove redundant InsecureSecretKey error variant - Downgrade synthetic template input log from debug! to trace! - Add build.rs #[path] guard comment to redacted.rs - Downgrade test log from info! to debug!

ChristianPavilonis · 2026-03-11T19:12:35Z

Review feedback addressed

All blocking and non-blocking items from the review have been resolved in 544d386:

Blocking fixes

1. PII/payload leak at WARN level in Prebid error path (prebid.rs:878-889)
Split the log: status code stays at warn!, response body preview moved to trace! behind a log_enabled! guard (avoids the from_utf8_lossy allocation in production).

2. Inconsistent secret sentinel checks (settings.rs + settings_data.rs + error.rs)
Consolidated both placeholder values ("secret_key" and "secret-key") into Synthetic::validate_secret_key. Removed the standalone check from settings_data.rs and the now-unused InsecureSecretKey error variant.

Non-blocking fixes

3. ValidationError codes (settings.rs:199-200)
Changed from human-readable messages to proper snake_case codes: "empty_secret_key", "placeholder_secret_key".

4. PII at debug level (synthetic.rs:99)
debug! → trace! for the template-rendered input containing raw IP/UA, consistent with the other payload downgrades in this PR.

5. Guard comment on redacted.rs (redacted.rs:1-2)
Added a comment noting the build.rs #[path] inclusion constraint so future contributors know to keep the file self-contained.

6. Test log level (synthetic.rs:266)
info! → debug! for the test log of the generated synthetic ID.

Deferred to follow-up issues

Make log level configurable via settings for incident/staging debugging #474 — Make log level configurable via settings for incident/staging debugging
Use constant-time comparison for basic auth password check #475 — Use constant-time comparison for basic auth password check
Add regression test asserting Settings debug output excludes secret values #476 — Add regression test asserting Settings debug output excludes secret values

All checks pass locally (fmt, clippy, 481 Rust tests, 239 JS tests, WASM build).

Redact secrets from logs and downgrade PII/payload log levels

5aae719

ChristianPavilonis self-assigned this Mar 10, 2026

ChristianPavilonis requested review from aram356 and prk-Jr March 10, 2026 15:20

Merge branch 'main' into fix/secrets-logged

a4ee384

aram356 requested changes Mar 11, 2026

View reviewed changes

This was referenced Mar 11, 2026

Make log level configurable via settings for incident/staging debugging #474

Open

Use constant-time comparison for basic auth password check #475

Open

Add regression test asserting Settings debug output excludes secret values #476

Open

ChristianPavilonis requested a review from aram356 March 11, 2026 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redact secrets from logs and downgrade PII log levels#468

Redact secrets from logs and downgrade PII log levels#468
ChristianPavilonis wants to merge 3 commits intomainfrom
fix/secrets-logged

ChristianPavilonis commented Mar 10, 2026 •

edited

Loading

Uh oh!

aram356 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis commented Mar 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ChristianPavilonis commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Closes

Test plan

Checklist

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

Non-blocking

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review feedback addressed

Blocking fixes

Non-blocking fixes

Deferred to follow-up issues

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ChristianPavilonis commented Mar 10, 2026 •

edited

Loading

ChristianPavilonis commented Mar 11, 2026 •

edited

Loading