Repository navigation
Do not inject unfilled placeholder values as credentials - #307
Merged
Merged
Conversation
The shipped workflow-config.json carries "github": {"token":
"YOUR_GITHUB_TOKEN"}. load_workflow_config injected that literal string into
GITHUB_TOKEN, and one placeholder then defeated BOTH GitHub backends at once.
The MCP client authenticated with it and got 401 Bad credentials. The gh CLI
fallback also lost, because gh prefers an environment token over its own keyring
and reported itself unauthenticated. So `gh auth login` appeared to do nothing:
the machine was correctly authenticated, `gh auth status` said so, a standalone
GitHubOperationRouter said so -- and the pipeline still fell back to "no GitHub
backend available" on every run and could create neither an issue nor a branch.
It was worth a day. The engine's whole SDLC arc was blocked by a string that was
never filled in.
The scope is wider than GitHub, which only became visible once the skip was
announced: this config was injecting TEN placeholder values -- anthropic.api_key,
figma.access_token, figma.team_id, github.token, jenkins.api_token, jenkins.url,
jenkins.user, jira.api_token, jira.url, jira.user. Anyone enabling any of those
integrations would have met the same failure, and the ones that take a URL rather
than a secret would have failed even less legibly.
The predicate is deliberately narrow, and half the tests exist to hold it there.
A false positive rejects a working secret and breaks a setup that was fine, which
is worse than the bug it guards against, so nothing is matched on length, entropy
or character class -- only on shapes no real credential has: a YOUR_/your- prefix,
angle brackets, CHANGEME, REPLACE_ME, TODO, xxx, or empty. ghp_/gho_/github_pat_
values pass, and so does "yourcompany-prod-key", which starts with "your" but is
not a template.
The skip is announced on stderr. A value silently ignored is indistinguishable
from one that was never read, and the reader is someone wondering why their
config has no effect -- which is exactly the position this bug put its author in.
Proven against the real broken config rather than a fixture: the placeholder was
restored to ~/.claude/workflow-config.json, and with it present GITHUB_TOKEN is
no longer injected, MCP falls through to the gh keyring, PyGithub authenticates,
and both backends initialise. The file is left in that shipped state, since it is
now handled correctly and the notice tells the user which integrations are
unconfigured.
One test-harness note worth recording: _load_raw memoises into _CONFIG_CACHE on
first read, after which the `path` argument is silently ignored. The first draft
of these tests did not reset it, so three of them asserted against the
developer's own ~/.claude/workflow-config.json instead of their fixtures -- a
green suite proving nothing. The autouse fixture resets it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
The shipped
workflow-config.jsoncarries"github": {"token": "YOUR_GITHUB_TOKEN"}.load_workflow_configinjected that literal string intoGITHUB_TOKEN, and one placeholder defeated both GitHub backends at once:401 Bad credentialsghprefers an environment token over its own keyring and reported itself unauthenticatedSo
gh auth loginappeared to do nothing. The machine was correctly authenticated —gh auth statussaid so, a standaloneGitHubOperationRoutersaid so — and the pipeline still fell back tono GitHub backend availableon every run, creating neither an issue nor a branch.The engine's whole SDLC arc was blocked by a string nobody filled in.
The scope is wider than GitHub
This only became visible once the skip was announced. The same config was injecting ten placeholder values:
Anyone enabling any of those integrations meets the same failure — and the ones taking a URL rather than a secret would fail even less legibly.
The predicate is deliberately narrow
A false positive rejects a working secret and breaks a setup that was fine — worse than the bug it guards against. So nothing is matched on length, entropy or character class, only on shapes no real credential has: a
YOUR_/your-prefix, angle brackets,CHANGEME,REPLACE_ME,TODO,xxx, or empty.Half the tests exist to hold it there.
ghp_…,gho_…,github_pat_…pass, and so doesyourcompany-prod-key— which starts with "your" but is not a template.The skip is announced on stderr: a value silently ignored is indistinguishable from one never read, and the reader is someone wondering why their config has no effect — exactly the position this bug put its author in.
Proven against the real broken config, not a fixture
The placeholder was restored to
~/.claude/workflow-config.jsonto reproduce the original state. With it present:The file is left in that shipped state, since it is now handled correctly and the notice tells the user which integrations are unconfigured.
A test-harness note worth recording
_load_rawmemoises into_CONFIG_CACHEon first read, after which thepathargument is silently ignored. The first draft of these tests did not reset it, so three of them asserted against the developer's own~/.claude/workflow-config.jsoninstead of their fixtures — a green suite proving nothing. The autouse fixture resets it, and the docstring says why.Context
This came out of the first successful end-to-end run of the pipeline: issue #306 and branch
feature/issue-306were minted for real once the placeholder was removed by hand. This PR makes that removal unnecessary.🤖 Generated with Claude Code