Skip to content

Logger: add Pino built-in redact as a defense-in-depth log-line safety net #3213

Description

@samayer12

Describe what should be investigated or refactored

Pepr configures Pino without using Pino's built-in redact option (path-based redaction with censor/remove). Instead, redaction is implemented manually in redactedStore and redactedPatch in src/lib/telemetry/logger.ts, gated on the PEPR_STORE_REDACT_VALUES env var.

This is a reasonable design as it stands: those functions redact store data structures before they are ever passed to the logger — a different concern than redacting fields on the emitted log line. They are not a general-purpose log-redaction mechanism.

The gap: because Pino's built-in redact safety net is not configured, there is no log-line-level guard against sensitive data reaching the logs through other call sites (e.g. any Log.info(obj) / Log.error(err, ...) elsewhere in the codebase that happens to carry secret-bearing objects). Redaction today only covers the store/patch paths that explicitly route through redactedStore/redactedPatch.

If log-line-level redaction of secret-bearing objects (Secrets, tokens, data fields, auth headers, etc.) is ever a requirement, Pino's redact paths would be the idiomatic tool — a single declarative config on the logger that applies to every log line regardless of call site.

Suggested investigation:

  • Decide whether log-line-level redaction is a requirement (threat model: what secret-bearing objects could plausibly be logged?).
  • If so, add a redact config to the Pino instance (paths + censor/remove) as a defense-in-depth layer alongside the existing store/patch redaction.
  • Clarify (in code comments/docs) that redactedStore/redactedPatch are store-data concerns, not a logger redaction mechanism, to avoid a false sense of coverage.

Links to any relevant code

  • Logger construction (no redact option):
    const Log = pino({
    transport,
    timestamp: pinoTimeFunction,
    });
  • Manual redactedStore / redactedPatch (gated on PEPR_STORE_REDACT_VALUES):
    export function redactedStore(store: Store): Store {
    const redacted = process.env.PEPR_STORE_REDACT_VALUES === "true";
    return {
    ...store,
    data: Object.keys(store.data).reduce((acc: Record<string, string>, key: string) => {
    acc[key] = redacted ? redactedValue : store.data[key];
    return acc;
    }, {}),
    };
    }
    export function redactedPatch(patch: Record<string, Operation> = {}): Record<string, Operation> {
    const redacted = process.env.PEPR_STORE_REDACT_VALUES === "true";
    if (!redacted) {
    return patch;
    }
    const redactedCache: Record<string, Operation> = {};
    Object.entries(patch).forEach(([key, operation]) => {
    const isRedacted = key.includes(":");
    const targetKey = isRedacted ? `${key.substring(0, key.lastIndexOf(":"))}:**redacted**` : key;
    const redactedOperation = isRedacted
    ? {
    ...operation,
    ...(Object.hasOwn(operation, "value") ? { value: redactedValue } : {}),
    }
    : operation;
    redactedCache[targetKey] = redactedOperation;
    });
    return redactedCache;
    }

Additional context

  • Pino redaction docs: https://getpino.io/#/docs/redaction
  • This is not a known active leak — it is a defense-in-depth / safety-net gap. The existing store/patch redaction works for its intended scope; this issue is about whether to also guard the log line itself against other call sites.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions