fix(config): refuse to write secret keys to git-tracked config.yaml#3652
Conversation
P0 security fix. bd config set for secret keys (api_key, secret, token, password) now detects git tracking of the target config file and refuses the write with an actionable error pointing to env var alternatives. Adds --force-git-tracked escape hatch for tests. Updates stale template documentation and setup guides to lead with env var approach.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
maphew
left a comment
There was a problem hiding this comment.
Found two issues that should be fixed before merge:
-
cmd/bd/config.go:114:config setrunsCheckSecretKeyGitSafetybefore checkingIsYamlOnlyKey, so DB-backed secret-looking keys likenotion.tokenare refused whenever.beads/config.yamlis tracked, even though they would not write toconfig.yaml.config set-manyonly checksyamlPairs, so the behavior is inconsistent. Move the check inside the YAML-only branch or makeCheckSecretKeyGitSafetyignore non-YAML keys. -
internal/config/yaml_config.go:126:secretKeyEnvVarHintgenerates generic env vars likeAI_API_KEYforai.api_key, but the code paths that use that config readANTHROPIC_API_KEY. For a tracked config, the remediation message can tell users to set an env var Beads does not consume. This needs either key-specific env var mapping or the message should avoid suggesting unsupported env vars.
Verification performed locally:
go test ./internal/configpassed.go test ./cmd/bd -run 'Test.*Config|Test.*Notion|Test.*Linear'could not build in this environment because ICU headers are missing:unicode/uregex.h.
Use real environment variable hints for known secret config keys and avoid blocking database-backed secret-looking keys such as notion.token. Maintainer follow-up for PR gastownhall#3652.
maphew
left a comment
There was a problem hiding this comment.
Maintainer follow-up pushed to the contributor branch in b3ab06b54 per PR_MAINTAINER_GUIDELINES.md instead of leaving this blocked on review rounds.
The follow-up addresses the two findings from my prior review:
- Secret git-tracking safety now ignores non-YAML-backed secret-looking keys such as
notion.token, avoiding theconfig set/config set-manyinconsistency. - Env var remediation now uses real known env vars (
LINEAR_API_KEY,GITHUB_TOKEN,ANTHROPIC_API_KEY) and falls back toBD_...for Viper-backed YAML config keys.
Local verification:
gofmton touched filesgit diff --checkgo test ./internal/config
Full PR CI is running on the new head commit.
The map contains environment variable names, not secret values.
Summary
P0 security fix.
bd config setfor secret keys (api_key,secret,token,password) now detects whether the targetconfig.yamlis tracked by git and refuses the write with an actionable error message.This addresses the verified credential-leak-on-default-path: the documented setup (
bd config set linear.api_key ...) writes to.beads/config.yamlwhich is git-tracked by default, leaking Linear API keys into git history.Changes
internal/config/yaml_config.go— AddedIsSecretKey,isGitTracked,CheckSecretKeyGitSafetyfunctions. Secret key detection uses substring matching againstapi_key,secret,token,password. Git tracking detection usesgit ls-files --error-unmatch.cmd/bd/config.go— Wired the safety check intoconfig setandconfig set-many. Added--force-git-trackedescape hatch flag.cmd/bd/init_templates.go— Fixed stale comment that incorrectly claimed secrets are "stored in the database" — now documents they're in config.yaml and recommends env vars.examples/linear-workflow/README.md— Replacedbd config set linear.api_keywithexport LINEAR_API_KEY=...docs/CONFIG.md— Linear setup section now leads with env var approach.Test plan
internal/configtest suite passes (0 regressions)go vetclean--force-git-trackedescape hatch works for test environmentsBackward compatibility
linear.team_ids,linear.priority_map.*, etc.) are unaffected--force-git-trackedprovides an opt-out for CI/test environmentsCharter compliance
Security hygiene of an existing flow. No new features, no new config surface beyond the
--force-git-trackedflag.Related
Need help on this PR? Tag
@codesmithwith what you need.