Do not inject unfilled placeholder values as credentials

CLAUDE.md

@@ -20,8 +20,8 @@ Claude Workflow Engine is a 3-level LangGraph-based orchestration pipeline for a
 | **Status** | Active Development |
 | **Primary Location** | langgraph_engine/ |
 | **MCP Servers** | 13 servers -- all in separate repos under [techdeveloper-org](https://github.com/orgs/techdeveloper-org/repositories); 1 also keeps an in-engine copy in `src/mcp/` |
-| **Total Python Files** | 248 (langgraph_engine/); 492 repo-wide |
-| **Test Files** | 104 (95 unit, 5 integration, 3 e2e, 1 load) |
+| **Total Python Files** | 248 (langgraph_engine/); 493 repo-wide |
+| **Test Files** | 105 (96 unit, 5 integration, 3 e2e, 1 load) |
 | **Call Graph** | 578 classes, 3,985 methods, 4 languages (Python/Java/TS/Kotlin) |
 
 ---

README.md

@@ -6,7 +6,7 @@
 [![Python](https://img.shields.io/badge/Python-3.10%2B-green)](https://python.org)
 [![CI](https://github.com/techdeveloper-org/claude-workflow-engine/actions/workflows/ci.yml/badge.svg)](https://github.com/techdeveloper-org/claude-workflow-engine/actions/workflows/ci.yml)
 [![License](https://img.shields.io/badge/License-MIT-yellow)](LICENSE)
-[![Tests](https://img.shields.io/badge/Tests-104%20files%20·%205%20integration-brightgreen)](tests/)
+[![Tests](https://img.shields.io/badge/Tests-105%20files%20·%205%20integration-brightgreen)](tests/)
 [![Discussions](https://img.shields.io/badge/Discussions-GitHub-blueviolet)](../../discussions)
 
 ---
@@ -469,8 +469,8 @@ claude-workflow-engine/           # 369 Python files total
 ├── policies/                     # non-.md policy state only (failure-kb.json) — see note below
 │   └── 03-execution-system/failure-prevention/failure-kb.json
 │
-├── tests/                        # 104 test_*.py files (125 total Python files)
-│   ├── test_*.py                 # 95 unit tests
+├── tests/                        # 105 test_*.py files (126 total Python files)
+│   ├── test_*.py                 # 96 unit tests
 │   ├── integration/              # 5 integration tests (GitHub, MCP, runtime verification)
 │   ├── e2e/                      # 3 end-to-end scenario tests
 │   └── load/                     # 1 concurrency / load test
@@ -628,11 +628,11 @@ ENABLE_HEALTH_SERVER=1 ENABLE_METRICS=1 LOG_FORMAT=json \
 
 ## Testing
 
-104 test_*.py files (125 total Python files in `tests/` including conftest and `__init__` files):
+105 test_*.py files (126 total Python files in `tests/` including conftest and `__init__` files):
 
 | Category | Files | Notes |
 |---|:---:|---|
-| Unit tests | 95 | In `tests/test_*.py` — no external dependencies required |
+| Unit tests | 96 | In `tests/test_*.py` — no external dependencies required |
 | Integration tests | 5 | `tests/integration/` — require live GitHub token or MCP servers |
 | E2E tests | 3 | `tests/e2e/` — require full pipeline environment |
 | Load tests | 1 | `tests/load/` — enabled with `RUN_LOAD_TESTS=1` |

langgraph_engine/core/config_loader.py

@@ -100,12 +100,54 @@ def _flatten(data: dict, prefix: str = "") -> dict[str, str]:
     return out
 
 
+# Values that mean "you were supposed to fill this in". The shipped
+# workflow-config.json carries them, so an untouched config injects them exactly
+# as if they were real credentials.
+#
+# This is deliberately narrow. A false positive here 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 real GitHub token starts ghp_/gho_/github_pat_ and a real
+# API key is not spelled CHANGEME.
+_PLACEHOLDER_PREFIXES = ("your_", "your-", "my_", "replace_", "replaceme", "changeme", "change_me", "todo", "xxx")
+
+
+def _is_placeholder(value: str) -> bool:
+    """Return True when a config value is an unfilled template, not a secret.
+
+    Args:
+        value: The raw value read from workflow-config.json.
+
+    Returns:
+        bool: True if the value should be treated as absent.
+    """
+    if not isinstance(value, str):
+        return False
+    stripped = value.strip()
+    if not stripped:
+        return True
+    if stripped.startswith("<") and stripped.endswith(">"):
+        return True
+    return stripped.lower().startswith(_PLACEHOLDER_PREFIXES)
+
+
 def load_workflow_config(path: Path | None = None) -> dict[str, str]:
     """Load workflow-config.json and inject into os.environ (no-override).
 
     Covers any remaining os.environ.get() call sites that have not yet
     been migrated to use get_section() directly.
 
+    Unfilled template values are skipped rather than injected. The reason is
+    specific and was expensive: the shipped config carries
+    ``"github": {"token": "YOUR_GITHUB_TOKEN"}``, this function injected it into
+    GITHUB_TOKEN, and that one string then defeated BOTH GitHub backends at once
+    -- the MCP client authenticated with it and got 401 Bad credentials, and the
+    gh CLI fallback also lost, because gh prefers an environment token over its
+    own keyring and reported itself unauthenticated. `gh auth login` therefore
+    appeared to do nothing, and Step 2 fell back to "no GitHub backend available"
+    on every run. Injecting nothing leaves the keyring reachable, which is the
+    working path.
+
     Returns a dict of env-var-name -> value pairs that were injected.
     """
     raw = _load_raw(path)
@@ -114,10 +156,27 @@ def load_workflow_config(path: Path | None = None) -> dict[str, str]:
 
     flat = _flatten(raw)
     injected: dict[str, str] = {}
+    skipped: list[str] = []
 
     for json_key, env_key in _MAPPING.items():
-        if json_key in flat and env_key not in os.environ:
-            os.environ[env_key] = flat[json_key]
-            injected[env_key] = flat[json_key]
+        if json_key not in flat or env_key in os.environ:
+            continue
+        value = flat[json_key]
+        if _is_placeholder(value):
+            skipped.append(json_key)
+            continue
+        os.environ[env_key] = value
+        injected[env_key] = value
+
+    # Said out loud, because a value silently ignored is indistinguishable from a
+    # value that was never read -- and the reader is someone wondering why their
+    # config has no effect.
+    if skipped:
+        import sys  # noqa: PLC0415
+
+        print(
+            "[config_loader] ignored %d unfilled placeholder value(s): %s" % (len(skipped), ", ".join(sorted(skipped))),
+            file=sys.stderr,
+        )
 
     return injected

tests/test_config_loader_placeholders.py

@@ -0,0 +1,156 @@
+"""An unfilled config template must not be injected as if it were a credential.
+
+The shipped workflow-config.json carries `"github": {"token": "YOUR_GITHUB_TOKEN"}`.
+load_workflow_config injected that 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, and the gh CLI fallback also lost, because gh prefers
+an environment token over its own keyring and reported itself unauthenticated.
+
+`gh auth login` therefore appeared to do nothing. Step 2 fell back to "no GitHub
+backend available" on every run, and the pipeline could not create an issue or a
+branch on a machine that was, by every other measure, correctly authenticated.
+Injecting nothing leaves the keyring reachable, which is the working path.
+
+The predicate is deliberately narrow, and half of these tests exist to hold it
+that way. A false positive rejects a real secret and breaks a setup that was
+working -- worse than the bug it guards against -- so nothing is matched on
+length, entropy or character class.
+"""
+
+import json
+import sys
+from pathlib import Path
+
+import pytest
+
+_PROJECT_ROOT = Path(__file__).resolve().parent.parent
+if str(_PROJECT_ROOT) not in sys.path:
+    sys.path.insert(0, str(_PROJECT_ROOT))
+
+from langgraph_engine.core import config_loader  # noqa: E402
+from langgraph_engine.core.config_loader import _is_placeholder, load_workflow_config  # noqa: E402
+
+
+@pytest.fixture(autouse=True)
+def _clean_env(monkeypatch):
+    """Reset the env vars and the module's config cache before each test.
+
+    _load_raw memoises into _CONFIG_CACHE on first read, after which the `path`
+    argument is silently ignored. Without this reset the first test to run would
+    pin the real ~/.claude/workflow-config.json for every test after it, and the
+    suite would be asserting against the developer's own machine -- which is how
+    a green suite ends up proving nothing.
+    """
+    for var in ("GITHUB_TOKEN", "GITHUB_DEFAULT_LABEL", "CLAUDE_GITHUB_OWNER", "JIRA_API_TOKEN"):
+        monkeypatch.delenv(var, raising=False)
+    monkeypatch.setattr(config_loader, "_CONFIG_CACHE", None)
+
+
+def _write_config(tmp_path, payload):
+    """Write a workflow-config.json and return its path."""
+    path = tmp_path / "workflow-config.json"
+    path.write_text(json.dumps(payload), encoding="utf-8")
+    return path
+
+
+class TestThePlaceholderIsNotInjected:
+    """The exact failure that cost a day."""
+
+    def test_the_shipped_github_placeholder_is_skipped(self, tmp_path):
+        path = _write_config(tmp_path, {"github": {"token": "YOUR_GITHUB_TOKEN"}})
+        injected = load_workflow_config(path)
+        assert "GITHUB_TOKEN" not in injected
+        assert "GITHUB_TOKEN" not in __import__("os").environ
+
+    def test_a_real_looking_token_is_still_injected(self, tmp_path):
+        """The half that fails if the predicate became greedy.
+
+        Without this, a predicate that rejected everything would satisfy the test
+        above -- and would break every correctly configured machine instead.
+        """
+        path = _write_config(tmp_path, {"github": {"token": "ghp_16CharsOrMoreOfRealLookingSecret"}})
+        injected = load_workflow_config(path)
+        assert injected.get("GITHUB_TOKEN") == "ghp_16CharsOrMoreOfRealLookingSecret"
+
+    def test_other_real_values_survive_alongside_a_skipped_one(self, tmp_path):
+        """One placeholder must not suppress the rest of the file."""
+        path = _write_config(
+            tmp_path,
+            {"github": {"token": "YOUR_GITHUB_TOKEN", "owner": "techdeveloper-org", "default_label": "feature"}},
+        )
+        injected = load_workflow_config(path)
+        assert "GITHUB_TOKEN" not in injected
+        assert injected.get("CLAUDE_GITHUB_OWNER") == "techdeveloper-org"
+        assert injected.get("GITHUB_DEFAULT_LABEL") == "feature"
+
+
+class TestTheSkipIsAnnounced:
+    """A value silently ignored is indistinguishable from one never read."""
+
+    def test_the_skip_is_reported(self, tmp_path, capsys):
+        path = _write_config(tmp_path, {"github": {"token": "YOUR_GITHUB_TOKEN"}})
+        load_workflow_config(path)
+        assert "github.token" in capsys.readouterr().err
+
+    def test_nothing_is_reported_when_nothing_was_skipped(self, tmp_path, capsys):
+        """The half that fails if the notice were unconditional."""
+        path = _write_config(tmp_path, {"github": {"owner": "techdeveloper-org"}})
+        load_workflow_config(path)
+        assert "config_loader" not in capsys.readouterr().err
+
+
+class TestThePredicateIsNarrow:
+    """A false positive breaks a working setup, so the shapes are enumerated."""
+
+    @pytest.mark.parametrize(
+        "value",
+        [
+            "YOUR_GITHUB_TOKEN",
+            "your-api-key",
+            "<your-token-here>",
+            "CHANGEME",
+            "change_me",
+            "REPLACE_ME",
+            "TODO",
+            "xxxxxxxx",
+            "",
+            "   ",
+        ],
+    )
+    def test_template_shapes_are_placeholders(self, value):
+        assert _is_placeholder(value) is True
+
+    @pytest.mark.parametrize(
+        "value",
+        [
+            "ghp_realLookingToken1234567890",
+            "gho_anotherRealShape",
+            "github_pat_11ABCDEF",
+            "techdeveloper-org",
+            "feature",
+            "sk-ant-api03-abcdef",
+            "yourcompany-prod-key",  # starts with "your" but not "your_" or "your-"
+            "0123456789abcdef",
+        ],
+    )
+    def test_real_values_are_not_placeholders(self, value):
+        assert _is_placeholder(value) is False
+
+    def test_a_non_string_is_not_a_placeholder(self):
+        """Config values may be bools or numbers; those are not templates."""
+        assert _is_placeholder(True) is False
+        assert _is_placeholder(0) is False
+
+
+class TestExistingBehaviourIsUnchanged:
+    """The no-override rule and missing-file handling must not have moved."""
+
+    def test_an_existing_env_var_still_wins(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("CLAUDE_GITHUB_OWNER", "already-set")
+        path = _write_config(tmp_path, {"github": {"owner": "from-config"}})
+        injected = load_workflow_config(path)
+        assert "CLAUDE_GITHUB_OWNER" not in injected
+        assert __import__("os").environ["CLAUDE_GITHUB_OWNER"] == "already-set"
+
+    def test_a_missing_file_returns_empty(self, tmp_path):
+        assert load_workflow_config(tmp_path / "absent.json") == {}