CLI exec sanitize secrets too - #6877
Conversation
The agent masks secret values before streaming step logs, but local `woodpecker exec` wrote step output to the terminal raw, leaking secrets passed via --secrets/--secrets-file. Add a reusable io.Writer decorator (shared.NewSecretsWriter) that masks secrets before forwarding, and wrap the exec log writer with it.
Run a dummy-backend pipeline with --secrets and assert the secret value never reaches the terminal output while the masked form does. Fails on the unfixed LineWriter, passes with the secrets writer in place. Co-authored-by: Codex <codex@openai.com>
Fprintf errors (e.g. broken pipe on stderr) were silently swallowed, misreporting successful writes upstream. Return the error and report zero bytes consumed. Make the output writer injectable for testing. Adopted from an external patch suggestion. Co-authored-by: Codex <codex@openai.com>
Generalize the secrets writer into shared.NewSanitizeWriter(dst, fn): an io.Writer decorator taking a pluggable SanitizeFunc, so the masking algorithm can be swapped later without touching writers. shared.NewSecretsWriter stays as the convenience wrapper using the secrets replacer. Move the agent onto the same decorator: strip the built-in replacer from the agent LineWriter and wrap its log stream with the shared secrets writer, matching the cli exec path. One masking implementation for both.
|
Surge PR preview deployment succeeded. View it at https://woodpecker-ci-woodpecker-pr-6877.surge.sh |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6877 +/- ##
==========================================
+ Coverage 48.45% 48.53% +0.08%
==========================================
Files 443 445 +2
Lines 29955 29977 +22
==========================================
+ Hits 14514 14549 +35
+ Misses 14215 14209 -6
+ Partials 1226 1219 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This looks good to me - but do we really need a separate |
|
I was looking into getting rid of sendingval secrets to agents to sanitize things |
|
still if you you want to i'll remove it and stick those changes into anotherbpull |
|
The comment was not about the structure of the PR, but the code only. You add a |
4 participants
the
cli execshould mimic server-agent execution, and share as much code as possible ...... this addresses secrets masking and refactor it a bit to make it more modular