kubernetes / kubernetes/git-sync
Secret-bearing flag defaults (--credential, --repo, --askpass-url) are printed in cleartext in usage output
- Dominant language
- Shell
- Stars
- 2.7k
- Forks
- 469
- PR merge metrics
- No merged PRs in 30d
Description
### What happened
`--credential`, `--repo` and `--askpass-url` take their default value from an environment
variable that can carry a secret. pflag prints a flag's default in usage output, so those
values appear in cleartext in `--help` and in the usage block printed on a flag parse error.
On master (`main.go:276`):
```
$ GITSYNC_CREDENTIAL='[{"url":"https://github.com","username":"bot","password":"SENTINEL_PAT"}]' \
git-sync --nonexistent-flag
...
--credential credentialSlice one or more credentials (see --man for details)
available for authentication
(default [{"url":"https://github.com","username":"bot",
"password":"SENTINEL_PAT"}])
```
Same shape for `--repo` with an embedded PAT:
```
$ GITSYNC_REPO='https://user:REPOPAT_xyz789@github.com/o/r.git' git-sync --help
--repo string the git repository to sync (required)
(default "https://user:REPOPAT_xyz789@github.com/o/r.git")
```
`--man` and `--version` are unaffected. Usage goes to stderr (`main.go:402`), except the
explicit `--help` path which uses stdout (`main.go:421`). Since git-sync usually runs as a
sidecar, both end up in the pod log, so a mistyped or stale flag in a manifest puts the
value into the log stream.
### Why I think it is worth a small fix
The startup log line is already clean. `logSafeFlags` (`main.go:1217-1258`) replaces
`cred.Password` with `REDACTED`, redacts `--password`, and runs `--repo` through
`redactURL`. I checked at `-v=4` that the startup line emits no secret, and at default
verbosity the flag is skipped entirely because an env-supplied default never sets `Changed`.
So the usage path looks like the one remaining gap in a control that already exists.
Two bits of history suggest this was scope rather than intent:
- `162e543` ("Add --credential flag to spec multiple user/pass", #803) added the
`envString`-as-default registration and the `cred.Password = redactedString` branch in
`logSafeFlags` in the same diff. Redaction was applied to the log line; usage output was
not in view at that moment.
- #886 ("Add the idea of env-flags") introduced `envFlag`, described at `env.go:323-326` as
"useful for things like passwords, which should not be on the CLI because it can be seen
in `ps`". `GITSYNC_PASSWORD` (`main.go:270`) and `GITSYNC_GITHUB_APP_PRIVATE_KEY`
(`main.go:299`) use `envFlagString` and do not leak. `--credential` stayed on
`envString`-as-default.
Related and previously fixed for the log path only: #602 and #851.
### Possible fix
pflag prints `Flag.DefValue`, a string captured at registration, rather than re-serialising
the value, so masking it after registration leaves the parsed value untouched:
```go
for _, name := range []string{"credential", "repo", "askpass-url"} {
if f := pflag.CommandLine.Lookup(name); f != nil && f.DefValue != "" {
f.DefValue = ""
}
}
```
Alternatives: move `--credential` to the existing `envFlag` mechanism to match `--password`
(though that changes whether it is settable on the CLI, which may not be wanted), or apply
the `logSafeFlags` redaction inside a custom usage function so both output paths share one
implementation.
Upstream does not offer a knob here: spf13/pflag#204 (`HideDefaultValue`) is unmerged and
go.mod pins pflag v1.0.5.
Happy to send a PR in whichever shape you prefer.
### On reporting channel
I am filing this in the open rather than through the Kubernetes security process because it
needs an operator mistake to trigger, not an attacker action, and because the security team
previously declined git-sync findings in this configuration-surface class as not crossing
the bar. If you would rather it went through the security process instead, say so and I will
move it.
### Version
master as of 2026-08-28, `main.go:276` unchanged. Reproduced from a fresh clone with a
locally built binary.
Contributor guide
Research direction
Start in main.go around flag registration at lines 270-299 and the usage paths at lines 402 and 421; check how pflag uses each flag's DefValue. Reproduce with the credential, repo, and askpass-url environment variables, then verify that help and parse-error output no longer contain their secret values while normal parsed values and existing redacted startup logging remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100