Skip to content

feat(config): enable redact_secrets by default#2889

Merged
dgageot merged 1 commit into
docker:mainfrom
rumpl:feat/redact-secrets-default-on
May 24, 2026
Merged

feat(config): enable redact_secrets by default#2889
dgageot merged 1 commit into
docker:mainfrom
rumpl:feat/redact-secrets-default-on

Conversation

@rumpl
Copy link
Copy Markdown
Member

@rumpl rumpl commented May 24, 2026

Make the agent-level redact_secrets flag opt-out instead of opt-in.

  • pkg/config/latest: field is now *bool; nil means enabled. New RedactSecretsEnabled() helper centralises the default.
  • pkg/teamloader: uses the helper when wiring agent.WithRedactSecrets.
  • agent-schema.json: default: true and updated description.
  • Examples updated to call out the new default; redact_secrets: false opts out.

Existing configs that omit the field gain redaction automatically. Configs that explicitly set true/false are unchanged. Auto-injection still dedupes against manually-written hook entries, so the redact_secrets_hooks.yaml example keeps working.

Tests: new unit tests cover default/explicit-true/explicit-false and YAML round-trip; existing redact_secrets builtin + teamloader tests pass.

Change agent-level `redact_secrets` to a tri-state `*bool` so an
omitted field now defaults to true. Existing configs gain
redaction automatically; set `redact_secrets: false` to opt out.

- pkg/config/latest: pointer field + RedactSecretsEnabled() helper
- teamloader: call the helper when wiring the agent option
- agent-schema.json: add `default: true` and update description
- examples/redact_secrets*.yaml: note the new default
- pkg/config/latest: unit tests for the default and YAML round-trip
@rumpl rumpl requested a review from a team as a code owner May 24, 2026 07:22
@aheritier aheritier added area/config For configuration parsing, YAML, environment variables area/security Authentication, authorization, secrets, vulnerabilities kind/feat PR adds a new feature (maps to feat: commit prefix) labels May 24, 2026
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

Reviewed 6 changed files (183 diff lines). The bool → *bool tri-state pattern for RedactSecrets is correctly implemented:

  • RedactSecretsEnabled() correctly handles nil receiver and nil pointer (default-on).
  • The only production call site (teamloader.go:167) already uses RedactSecretsEnabled() — no unupdated callers.
  • pkg/runtime/hooks.go calls a.RedactSecrets() on *Agent (not *AgentConfig), which returns the already-resolved bool set by teamloader via WithRedactSecrets(agentConfig.RedactSecretsEnabled()) — correct downstream path.
  • omitempty on *bool correctly omits only nil (unset), not false (explicitly disabled).
  • Test coverage in redact_secrets_test.go covers nil cfg, nil field, explicit true, and explicit false cases.

No bugs found in the changed code.

@dgageot dgageot merged commit c291dfc into docker:main May 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config For configuration parsing, YAML, environment variables area/security Authentication, authorization, secrets, vulnerabilities kind/feat PR adds a new feature (maps to feat: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants