Skip to content

[hardening] Wrap ProviderKey.secret (+ guardrail/exporter creds) in a redacting Secret type — make log leakage impossible by construction #547

Description

@moonming

Summary

Provider/guardrail/exporter secrets are stored as plaintext String that derive Debug. Leakage to logs is prevented today only by discipline (an audit confirmed no current log statement prints them), not by construction. A future debug!("{:?}", provider_key) or any struct-dump would print the upstream secret into production logs. Wrapping secrets in a redacting newtype makes that a compile-time impossibility.

Current state

  • crates/aisix-core/src/models/provider_key.rs:
    #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)]
    pub struct ProviderKey { pub secret: String, /* … */ }
    Plaintext + Debug-derived.
  • Same trust boundary (per the secret field's own doc comment): Guardrail credentials (models/guardrail.rs secret_access_key / access_key_secret) and ObservabilityExporter headers (models/observability_exporter.rs).
  • Verified: no log statement currently prints these (so no active leak); protection is discipline-only.

Proposal — a redacting Secret newtype

Option A — secrecy crate (SecretString): zeroize-on-drop, battle-tested. Needs a serde-transparent wrapper for the etcd-projection round-trip (cp-api projects the plaintext secret over mTLS).

Option B — hand-rolled newtype (fits the existing serde-transparent need):

// crates/aisix-core/src/models/secret.rs
#[derive(Clone, Serialize, Deserialize, PartialEq, schemars::JsonSchema)]
#[serde(transparent)]                 // wire / on-disk / etcd stay a bare string
pub struct Secret(String);

impl std::fmt::Debug for Secret {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.write_str("Secret([redacted])")
    }
}
impl Secret {
    pub fn expose(&self) -> &str { &self.0 }  // sole plaintext accessor → greppable audit
}

Changes

  1. ProviderKey.secret: StringSecret.
  2. Every use site (bridges building the api-key / Authorization header, Bedrock SigV4 signer) → provider_key.secret.expose() — making every plaintext access explicit and grep-auditable.
  3. #[derive(Debug)] on ProviderKey is now safe by construction (prints Secret([redacted])).
  4. Extend to Guardrail (secret_access_key, access_key_secret) and ObservabilityExporter secret/header fields.

Non-breaking

#[serde(transparent)] keeps the wire / on-disk / etcd projection format a bare string — behavior unchanged; this only adds a Debug/Display guard.

Known boundary

This guards accidental Debug/Display logging, not a deliberate serde_json::to_string(&pk) written into a log (rarer path). Call that out in review / add a lint if desired.

Test

api7/ai-gateway has Rust unit tests; add:

#[test]
fn provider_key_debug_redacts_secret() {
    let pk = ProviderKey { secret: Secret("super-secret".into()), /* … */ };
    let d = format!("{:?}", pk);
    assert!(!d.contains("super-secret"));
    assert!(d.contains("[redacted]"));
}

Priority / sequencing

Defense-in-depth. No active leak today (verified), so not urgent — but it's the structural fix that makes the whole leakage class impossible. Pairs with the CI-side backstop already merged in api7/AISIX-Cloud#713 (test-harness redactSecrets for dumped docker logs). Suggested as a 2-step PR: (1) Secret newtype + ProviderKey; (2) extend to guardrail / observability creds.

Refs: api7/AISIX-Cloud#713 (test-harness redaction backstop, merged).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions