fix: scrub access token from logs on failure path - #47
Open
adamrtalbot wants to merge 1 commit into
Open
Conversation
The token scrub ran as the second-to-last statement in the script, so a non-zero `tw launch` aborted under `set -e` before reaching it. The log file was already on disk with the raw token, and CI uploads `tower_action_*.log` with `if: success() || failure()`. Move the scrub into a function armed as an EXIT trap so it runs on the abort path too, and cover the json file as well. The explicit call before `cat` stays because the trap fires after it. Also switch the sed delimiter from `/` to `|` so a `/` in a future token format cannot break the expression, and add `g` plus an empty-token guard. Generated by Claude Code
🚀 Pipelines launched
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
tower_action_*.logcould contain the Seqera access token in plain text, and CI uploads it as a workflow artifact even when the run fails.The scrub existed, but it was the second-to-last line of
entrypoint.sh:Under
set -euo pipefail, a non-zerotw launchkills the script at the launch line, ~35 lines earlier. The log is already written by then, so it ships unscrubbed..github/workflows/ci.ymluploads it onsuccess() || failure(), and CI deliberately launches a nonexistent pipeline to test the failure path.::add-mask::does not cover this: it redacts the live console, not files on disk.Fix
Run the scrub from an
EXITtrap, so it happens whether the script finishes or aborts. Also applied to the.jsonfile. The explicit call beforecatstays, because the trap fires after it.Minor hardening in the same
sed: delimiter/to|so a/in a future token format cannot break the expression, thegflag, and a guard for an empty token (wheres||xxxxxx|gwould insert the replacement between every character).Only the token is scrubbed. The other masked values are identifiers, and
workspaceIdis documented content of the.jsonartifact, so redacting it would break consumers.Secrets you interpolate into
nextflow_config,parametersorpre_run_scriptare still your problem: the entrypoint never sees those values.