|
| 1 | +"""An unfilled config template must not be injected as if it were a credential. |
| 2 | +
|
| 3 | +The shipped workflow-config.json carries `"github": {"token": "YOUR_GITHUB_TOKEN"}`. |
| 4 | +load_workflow_config injected that string into GITHUB_TOKEN, and one placeholder |
| 5 | +then defeated BOTH GitHub backends at once: the MCP client authenticated with it |
| 6 | +and got 401 Bad credentials, and the gh CLI fallback also lost, because gh prefers |
| 7 | +an environment token over its own keyring and reported itself unauthenticated. |
| 8 | +
|
| 9 | +`gh auth login` therefore appeared to do nothing. Step 2 fell back to "no GitHub |
| 10 | +backend available" on every run, and the pipeline could not create an issue or a |
| 11 | +branch on a machine that was, by every other measure, correctly authenticated. |
| 12 | +Injecting nothing leaves the keyring reachable, which is the working path. |
| 13 | +
|
| 14 | +The predicate is deliberately narrow, and half of these tests exist to hold it |
| 15 | +that way. A false positive rejects a real secret and breaks a setup that was |
| 16 | +working -- worse than the bug it guards against -- so nothing is matched on |
| 17 | +length, entropy or character class. |
| 18 | +""" |
| 19 | + |
| 20 | +import json |
| 21 | +import sys |
| 22 | +from pathlib import Path |
| 23 | + |
| 24 | +import pytest |
| 25 | + |
| 26 | +_PROJECT_ROOT = Path(__file__).resolve().parent.parent |
| 27 | +if str(_PROJECT_ROOT) not in sys.path: |
| 28 | + sys.path.insert(0, str(_PROJECT_ROOT)) |
| 29 | + |
| 30 | +from langgraph_engine.core import config_loader # noqa: E402 |
| 31 | +from langgraph_engine.core.config_loader import _is_placeholder, load_workflow_config # noqa: E402 |
| 32 | + |
| 33 | + |
| 34 | +@pytest.fixture(autouse=True) |
| 35 | +def _clean_env(monkeypatch): |
| 36 | + """Reset the env vars and the module's config cache before each test. |
| 37 | +
|
| 38 | + _load_raw memoises into _CONFIG_CACHE on first read, after which the `path` |
| 39 | + argument is silently ignored. Without this reset the first test to run would |
| 40 | + pin the real ~/.claude/workflow-config.json for every test after it, and the |
| 41 | + suite would be asserting against the developer's own machine -- which is how |
| 42 | + a green suite ends up proving nothing. |
| 43 | + """ |
| 44 | + for var in ("GITHUB_TOKEN", "GITHUB_DEFAULT_LABEL", "CLAUDE_GITHUB_OWNER", "JIRA_API_TOKEN"): |
| 45 | + monkeypatch.delenv(var, raising=False) |
| 46 | + monkeypatch.setattr(config_loader, "_CONFIG_CACHE", None) |
| 47 | + |
| 48 | + |
| 49 | +def _write_config(tmp_path, payload): |
| 50 | + """Write a workflow-config.json and return its path.""" |
| 51 | + path = tmp_path / "workflow-config.json" |
| 52 | + path.write_text(json.dumps(payload), encoding="utf-8") |
| 53 | + return path |
| 54 | + |
| 55 | + |
| 56 | +class TestThePlaceholderIsNotInjected: |
| 57 | + """The exact failure that cost a day.""" |
| 58 | + |
| 59 | + def test_the_shipped_github_placeholder_is_skipped(self, tmp_path): |
| 60 | + path = _write_config(tmp_path, {"github": {"token": "YOUR_GITHUB_TOKEN"}}) |
| 61 | + injected = load_workflow_config(path) |
| 62 | + assert "GITHUB_TOKEN" not in injected |
| 63 | + assert "GITHUB_TOKEN" not in __import__("os").environ |
| 64 | + |
| 65 | + def test_a_real_looking_token_is_still_injected(self, tmp_path): |
| 66 | + """The half that fails if the predicate became greedy. |
| 67 | +
|
| 68 | + Without this, a predicate that rejected everything would satisfy the test |
| 69 | + above -- and would break every correctly configured machine instead. |
| 70 | + """ |
| 71 | + path = _write_config(tmp_path, {"github": {"token": "ghp_16CharsOrMoreOfRealLookingSecret"}}) |
| 72 | + injected = load_workflow_config(path) |
| 73 | + assert injected.get("GITHUB_TOKEN") == "ghp_16CharsOrMoreOfRealLookingSecret" |
| 74 | + |
| 75 | + def test_other_real_values_survive_alongside_a_skipped_one(self, tmp_path): |
| 76 | + """One placeholder must not suppress the rest of the file.""" |
| 77 | + path = _write_config( |
| 78 | + tmp_path, |
| 79 | + {"github": {"token": "YOUR_GITHUB_TOKEN", "owner": "techdeveloper-org", "default_label": "feature"}}, |
| 80 | + ) |
| 81 | + injected = load_workflow_config(path) |
| 82 | + assert "GITHUB_TOKEN" not in injected |
| 83 | + assert injected.get("CLAUDE_GITHUB_OWNER") == "techdeveloper-org" |
| 84 | + assert injected.get("GITHUB_DEFAULT_LABEL") == "feature" |
| 85 | + |
| 86 | + |
| 87 | +class TestTheSkipIsAnnounced: |
| 88 | + """A value silently ignored is indistinguishable from one never read.""" |
| 89 | + |
| 90 | + def test_the_skip_is_reported(self, tmp_path, capsys): |
| 91 | + path = _write_config(tmp_path, {"github": {"token": "YOUR_GITHUB_TOKEN"}}) |
| 92 | + load_workflow_config(path) |
| 93 | + assert "github.token" in capsys.readouterr().err |
| 94 | + |
| 95 | + def test_nothing_is_reported_when_nothing_was_skipped(self, tmp_path, capsys): |
| 96 | + """The half that fails if the notice were unconditional.""" |
| 97 | + path = _write_config(tmp_path, {"github": {"owner": "techdeveloper-org"}}) |
| 98 | + load_workflow_config(path) |
| 99 | + assert "config_loader" not in capsys.readouterr().err |
| 100 | + |
| 101 | + |
| 102 | +class TestThePredicateIsNarrow: |
| 103 | + """A false positive breaks a working setup, so the shapes are enumerated.""" |
| 104 | + |
| 105 | + @pytest.mark.parametrize( |
| 106 | + "value", |
| 107 | + [ |
| 108 | + "YOUR_GITHUB_TOKEN", |
| 109 | + "your-api-key", |
| 110 | + "<your-token-here>", |
| 111 | + "CHANGEME", |
| 112 | + "change_me", |
| 113 | + "REPLACE_ME", |
| 114 | + "TODO", |
| 115 | + "xxxxxxxx", |
| 116 | + "", |
| 117 | + " ", |
| 118 | + ], |
| 119 | + ) |
| 120 | + def test_template_shapes_are_placeholders(self, value): |
| 121 | + assert _is_placeholder(value) is True |
| 122 | + |
| 123 | + @pytest.mark.parametrize( |
| 124 | + "value", |
| 125 | + [ |
| 126 | + "ghp_realLookingToken1234567890", |
| 127 | + "gho_anotherRealShape", |
| 128 | + "github_pat_11ABCDEF", |
| 129 | + "techdeveloper-org", |
| 130 | + "feature", |
| 131 | + "sk-ant-api03-abcdef", |
| 132 | + "yourcompany-prod-key", # starts with "your" but not "your_" or "your-" |
| 133 | + "0123456789abcdef", |
| 134 | + ], |
| 135 | + ) |
| 136 | + def test_real_values_are_not_placeholders(self, value): |
| 137 | + assert _is_placeholder(value) is False |
| 138 | + |
| 139 | + def test_a_non_string_is_not_a_placeholder(self): |
| 140 | + """Config values may be bools or numbers; those are not templates.""" |
| 141 | + assert _is_placeholder(True) is False |
| 142 | + assert _is_placeholder(0) is False |
| 143 | + |
| 144 | + |
| 145 | +class TestExistingBehaviourIsUnchanged: |
| 146 | + """The no-override rule and missing-file handling must not have moved.""" |
| 147 | + |
| 148 | + def test_an_existing_env_var_still_wins(self, tmp_path, monkeypatch): |
| 149 | + monkeypatch.setenv("CLAUDE_GITHUB_OWNER", "already-set") |
| 150 | + path = _write_config(tmp_path, {"github": {"owner": "from-config"}}) |
| 151 | + injected = load_workflow_config(path) |
| 152 | + assert "CLAUDE_GITHUB_OWNER" not in injected |
| 153 | + assert __import__("os").environ["CLAUDE_GITHUB_OWNER"] == "already-set" |
| 154 | + |
| 155 | + def test_a_missing_file_returns_empty(self, tmp_path): |
| 156 | + assert load_workflow_config(tmp_path / "absent.json") == {} |
0 commit comments