Skip to content

Don't allow secrets to be extracted without approval - #135

Merged
ldayton merged 2 commits into
ldayton:mainfrom
nickdavies:kubectl_secrets
Jun 8, 2026
Merged

Don't allow secrets to be extracted without approval#135
ldayton merged 2 commits into
ldayton:mainfrom
nickdavies:kubectl_secrets

Conversation

@nickdavies

Copy link
Copy Markdown
Contributor

Secrets -o yaml and config --raw could be leaking secrets that should be generally sealed in the cluster so don't allow those by default.

This also rethinks how sometimes-safe commands are treated: commands with opaque tokens (command substitutions, parameter expansions, indirect expansions) that could alter security-sensitive arguments are conservatively blocked. Handlers receive opaque_positions so they can detect when runtime-determined values bypass static checks.

I believe this is a sane pattern for handling this case. I did the same choice in the modules and -c PRs where if there are expansions we just default to unsafe. I think this is scoped down correctly though so that most get commands for kubectl (except for secret are going to be approved automatically still the false-negative should be low

Secrets -o yaml and config --raw are leaking secrets that should be
generally sealed in the cluster so don't allow those by default.

Also rethinks how sometimes-safe commands are treated: commands with
opaque tokens (command substitutions, parameter expansions, indirect
expansions) that could alter security-sensitive arguments are
conservatively blocked. Handlers receive opaque_positions so they can
detect when runtime-determined values bypass static checks.
ldayton#149 landed word_has_expansions (per-token: was the word built from a bash
expansion), which already does what this PR's opaque_positions needed and a
bit more — it also flags multi-expansion words like a$X$Y, and it's aligned
to the handler's tokens through env prefixes and wrapper recursion.

Resolve the conflict by dropping opaque_positions and pointing the kubectl
secret-exposure checks at word_has_expansions, so there's a single
expansion-tracking mechanism in the analyzer's security path instead of two
parallel ones. Behavior is equivalent-or-stricter; the kubectl tests are
unchanged and still pass.

Co-authored-by: Nick Davies <github@nicolasdavies.com.au>
@ldayton
ldayton merged commit 74e079e into ldayton:main Jun 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants