molecule-ai/molecule-ai-workspace-runtime
51
0
Fork 0
Code Issues 4 Pull Requests 1 Actions Packages Projects Releases 1 Wiki Activity

fix(runtime#86): re-apply GIT_ASKPASS + add workflow URL-embedding regression gate #167

Merged
hongming merged 2 commits from fix/86-git-askpass-no-token-in-argv into main 2026-06-23 08:05:43 +00:00
Conversation 0 Commits 2 Files Changed 4
+336 -26
agent-dev-b commented 2026-06-23 08:02:04 +00:00
Member
Copy Link
Copy Source

What

Closes runtime#86 end-to-end. Two commits:

  1. Re-applies the GIT_ASKPASS conversion for the audit scripts (check_consumer_runtime_drift.py, check_platform_comm_contract.py). Kimi's prior attempt (commit 061716f, 2026-06-07) was reverted twice on main (2ae28eb, 480045a — both bare git revert outputs with no commit-body explanation); the URL-embedded token pattern is back on main @ 924c0fd at scripts/check_consumer_runtime_drift.py:308 + scripts/check_platform_comm_contract.py:290. This commit puts the fix back and adds back the test_clone_consumers_never_puts_token_in_argv regression test.

  2. Adds a workflow-file grep gate (tests/test_workflow_no_token_in_url.py) that fails CI if the x-access-token:${TOKEN} URL-embedding pattern is re-introduced in any .gitea/workflows/*.yml or scripts/*.{py,sh}. The gate distinguishes the leak shape from the documented safe echo "x-access-token" username response in the GIT_ASKPASS helper body, and skips comment lines so the docstring of this file + future commit messages don't false-positive.

Why both pieces

The leak surface is two scripts (Python) plus two workflow files (CI). Re-applying the GIT_ASKPASS fix closes the live leak. The gate makes the closure durable — without it, the next PR that re-introduces the URL-embedded pattern passes CI and ships the leak back to production.

This is the same auth-leak family as runtime#161 (CLAUDE_CODE_OAUTH_TOKEN drain) and runtime#162 (ANTHROPIC_AUTH_TOKEN drain, just shipped as PR#165). Same shape: a secret ends up in a place where an attacker with log-read / process-inspection access can read it.

Diligence on the Kimi revert history (PM asked)

PM asked for a why-check before re-applying. Findings:

  • Both reverts (2ae28eb, 480045a) are bare git revert outputs with no commit-body explanation.
  • No comments on the runtime#86 issue discussing a real bug.
  • No related PRs around June 8 that would have surfaced a real conflict.
  • credential_helper work (runtime#104, commit fd2051d) sets git config --global credential.helper — a different mechanism, no overlap with GIT_ASKPASS. No conflict.
  • Re-read Kimi's 061716f implementation in full: standard GIT_ASKPASS pattern (subprocess.run with env override, 0o700 askpass, os.unlink in finally, shlex.quote on the token). Looks correct.
  • Smoke-tested the helper locally: writes a 0o700 script with the expected body, sets GIT_ASKPASS env, does not leak token or username into subprocess argv, and unlinks the file in the finally clause.

Verdict: the reverts appear to be accidental state-management (git revert then git revert <revert> then git revert again) rather than a discovered bug. The implementation is correct; the live leak was the worse outcome.

What the gate matches (forbidden)

x-access-token: followed by a token-looking value:

  1. ${VAR} — bash env-var interpolation
  2. ${{ ... }} — GitHub Actions expression
  3. {var} — Python f-string
  4. a quoted token literal (≥8 chars)

What the gate does NOT match (allowed)

  • The GIT_ASKPASS helper's echo "x-access-token" username response — the documented safe shape (the token rides the password prompt, not the URL). Pinned by test_askpass_username_response_is_the_only_allowed_occurrence so a refactor of the askpass helper doesn't silently break the gate.
  • Comment lines — the prior fix's commit message and this test file reference the pattern as documentation; comment-line skip is applied at the scan layer.

Tests

File Status
tests/test_llm_auth.py 35 (unrelated sanity)
tests/test_consumer_runtime_drift_guard.py 13 (12 existing + 1 restored from Kimi's commit)
tests/test_platform_comm_contract_guard.py 6 (existing)
tests/test_workflow_no_token_in_url.py 4 (new gate)
Total 58 pass

Positive control (manual, reproducible)

Inject into .gitea/workflows/publish-runtime.yml:

- run: git clone "https://x-access-token:${SECRET}@github.com/x/y"

Re-run pytest tests/test_workflow_no_token_in_url.py::test_no_workflow_or_script_embeds_token_in_clone_url -v → red, with a file:line-cited failure:

Forbidden token-in-URL pattern detected (runtime#86 regression). ...
Offenders:
  .gitea/workflows/publish-runtime.yml:161: - run: git clone "https://x-access-token:${SECRET}@github.com/x/y"

Revert the injection → green.

Smoke test for the helper

guard._git_clone_with_token(
    Path('/tmp/dest'),
    'https://git.moleculesai.app/molecule-ai/foo.git',
    's3cr3t-t0k3n',
)

Result: askpass file created with 0o700, body contains the expected echo "x-access-token" + echo 's3cr3t-t0k3n' (single-quoted via shlex.quote), token/username absent from subprocess argv, file unlinked in finally. ✓

Independence from the red #3164 deployment surface

Pure scripts + test surface. No concierge / MCP / heartbeat / identity-gate / operator-deployment touched. Safe to merge on the runtime-lane (CI-only gate; doesn't need the operator #3164 redeploy).

Gate

  • 2-genuine (CR2 + Researcher) — routing needed; PM asked for security review
  • CI green — unit-tests job + secret-scan job (this gate adds to the test surface)
  • target = main
## What Closes runtime#86 end-to-end. Two commits: 1. **Re-applies the GIT_ASKPASS conversion** for the audit scripts (`check_consumer_runtime_drift.py`, `check_platform_comm_contract.py`). Kimi's prior attempt (commit `061716f`, 2026-06-07) was reverted twice on main (`2ae28eb`, `480045a` — both bare `git revert` outputs with no commit-body explanation); the URL-embedded token pattern is back on `main @ 924c0fd` at `scripts/check_consumer_runtime_drift.py:308` + `scripts/check_platform_comm_contract.py:290`. This commit puts the fix back and adds back the `test_clone_consumers_never_puts_token_in_argv` regression test. 2. **Adds a workflow-file grep gate** (`tests/test_workflow_no_token_in_url.py`) that fails CI if the `x-access-token:${TOKEN}` URL-embedding pattern is re-introduced in any `.gitea/workflows/*.yml` or `scripts/*.{py,sh}`. The gate distinguishes the leak shape from the documented safe `echo "x-access-token"` username response in the GIT_ASKPASS helper body, and skips comment lines so the docstring of this file + future commit messages don't false-positive. ## Why both pieces The leak surface is **two scripts** (Python) plus **two workflow files** (CI). Re-applying the GIT_ASKPASS fix closes the live leak. The gate makes the closure durable — without it, the next PR that re-introduces the URL-embedded pattern passes CI and ships the leak back to production. This is the same auth-leak family as runtime#161 (`CLAUDE_CODE_OAUTH_TOKEN` drain) and runtime#162 (`ANTHROPIC_AUTH_TOKEN` drain, just shipped as PR#165). Same shape: a secret ends up in a place where an attacker with log-read / process-inspection access can read it. ## Diligence on the Kimi revert history (PM asked) PM asked for a why-check before re-applying. Findings: - Both reverts (`2ae28eb`, `480045a`) are bare `git revert` outputs with **no commit-body explanation**. - **No comments** on the runtime#86 issue discussing a real bug. - **No related PRs** around June 8 that would have surfaced a real conflict. - `credential_helper` work (runtime#104, commit `fd2051d`) sets `git config --global credential.helper` — a **different mechanism**, no overlap with GIT_ASKPASS. No conflict. - Re-read Kimi's 061716f implementation in full: standard GIT_ASKPASS pattern (`subprocess.run` with env override, 0o700 askpass, `os.unlink` in `finally`, `shlex.quote` on the token). Looks correct. - Smoke-tested the helper locally: writes a 0o700 script with the expected body, sets `GIT_ASKPASS` env, does not leak token or username into subprocess argv, and unlinks the file in the `finally` clause. **Verdict:** the reverts appear to be accidental state-management (`git revert` then `git revert <revert>` then `git revert` again) rather than a discovered bug. The implementation is correct; the live leak was the worse outcome. ## What the gate matches (forbidden) `x-access-token:` followed by a token-looking value: 1. `${VAR}` — bash env-var interpolation 2. `${{ ... }}` — GitHub Actions expression 3. `{var}` — Python f-string 4. a quoted token literal (≥8 chars) ## What the gate does NOT match (allowed) - The GIT_ASKPASS helper's `echo "x-access-token"` username response — the documented safe shape (the token rides the password prompt, not the URL). Pinned by `test_askpass_username_response_is_the_only_allowed_occurrence` so a refactor of the askpass helper doesn't silently break the gate. - Comment lines — the prior fix's commit message and this test file reference the pattern as documentation; comment-line skip is applied at the scan layer. ## Tests | File | Status | |---|---| | `tests/test_llm_auth.py` | 35 (unrelated sanity) | | `tests/test_consumer_runtime_drift_guard.py` | 13 (12 existing + 1 restored from Kimi's commit) | | `tests/test_platform_comm_contract_guard.py` | 6 (existing) | | `tests/test_workflow_no_token_in_url.py` | **4 (new gate)** | | **Total** | **58 pass** | ### Positive control (manual, reproducible) Inject into `.gitea/workflows/publish-runtime.yml`: ```yaml - run: git clone "https://x-access-token:${SECRET}@github.com/x/y" ``` Re-run `pytest tests/test_workflow_no_token_in_url.py::test_no_workflow_or_script_embeds_token_in_clone_url -v` → **red**, with a file:line-cited failure: ``` Forbidden token-in-URL pattern detected (runtime#86 regression). ... Offenders: .gitea/workflows/publish-runtime.yml:161: - run: git clone "https://x-access-token:${SECRET}@github.com/x/y" ``` Revert the injection → green. ### Smoke test for the helper ```python guard._git_clone_with_token( Path('/tmp/dest'), 'https://git.moleculesai.app/molecule-ai/foo.git', 's3cr3t-t0k3n', ) ``` Result: askpass file created with `0o700`, body contains the expected `echo "x-access-token"` + `echo 's3cr3t-t0k3n'` (single-quoted via `shlex.quote`), token/username absent from subprocess argv, file unlinked in `finally`. ✓ ## Independence from the red #3164 deployment surface Pure scripts + test surface. **No** concierge / MCP / heartbeat / identity-gate / operator-deployment touched. Safe to merge on the runtime-lane (CI-only gate; doesn't need the operator #3164 redeploy). ## Gate - [ ] 2-genuine (CR2 + Researcher) — routing needed; PM asked for security review - [ ] CI green — `unit-tests` job + `secret-scan` job (this gate adds to the test surface) - [ ] target = main
agent-dev-b added 2 commits 2026-06-23 08:02:05 +00:00
test(runtime#86): gate token-in-URL regressions in workflows + scripts 0aa2794936 
Closes the remaining acceptance-criteria gap from runtime#86: a
regression test that fails the build if the
x-access-token-in-clone-URL pattern is re-introduced in
.gitea/workflows/*.yml or scripts/*.{py,sh}.

Why this is the last piece
--------------------------
Kimi's prior commit (061716f) on this branch converted the Python audit
scripts to use GIT_ASKPASS so the token never lands in subprocess argv
or the git remote URL. The publish-runtime.yml cascade had ALREADY been
replaced (commits 28cbf9b, 7154e15) with a Gitea contents+pulls API
approach — no git clone, no on-disk remote URL with a token. So the
practical leak surface is closed; the missing piece is the regression
guard so the next PR that re-introduces the pattern goes red in CI
rather than red in production.

What the gate scans
-------------------
* .gitea/workflows/*.yml — the CI surface
* scripts/*.py — the audit-script surface (clone helpers used by CI)
* scripts/*.sh — shell helpers (none shipped; covered for symmetry)

What the gate matches
---------------------
x-access-token: followed by a token-looking value:
  1. bash ${VAR}
  2. GitHub Actions ${{ ... }}
  3. Python f-string {var}
  4. a quoted token literal (>=8 chars)

What the gate does NOT match
----------------------------
* The GIT_ASKPASS helper's 'echo "x-access-token"' username response
  (the token rides the password prompt, not the URL — the documented
  safe shape).
* Comment lines (the prior fix's commit message + this test file
  reference the pattern as documentation; the comment-line skip is
  applied at the scan layer).

Tests
-----
* test_scan_targets_exist — defence-in-depth, the gate scans
  something (vacuous-pass guard).
* test_no_workflow_or_script_embeds_token_in_clone_url — the main
  check.
* test_askpass_username_response_is_the_only_allowed_occurrence —
  pins the documented exception so a refactor of the askpass helper
  doesn't silently break the gate.
* test_leak_regex_does_not_match_comment_lines — regression guard
  for the gate itself (verifies the comment-skip is wired up).

Positive control (manual): injecting
  - run: git clone "https://x-access-token:${SECRET}@github.com/x/y"
into .gitea/workflows/publish-runtime.yml and re-running the test
produces a clear, file:line-cited failure:
  Forbidden token-in-URL pattern detected (runtime#86 regression). ...
  Offenders:
    .gitea/workflows/publish-runtime.yml:161: - run: git clone ...

Existing tests still pass:
  tests/test_llm_auth.py — 35/35
  tests/test_consumer_runtime_drift_guard.py — 6/6
  tests/test_platform_comm_contract_guard.py — 6/6
  tests/test_workflow_no_token_in_url.py — 4/4
Total 51/51.

Closes: runtime#86
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(runtime#86): re-apply GIT_ASKPASS conversion for the audit scripts
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 7s
Details
ci / lint (pull_request) Successful in 17s
Details
ci / build (pull_request) Successful in 35s
Details
ci / smoke-install (pull_request) Successful in 50s
Details
ci / unit-tests (pull_request) Successful in 1m16s
Details
ci / responsiveness-e2e (pull_request) Successful in 1m44s
Details
4e01b29336 
Re-applies Kimi's prior GIT_ASKPASS conversion (originally 061716f, reverted
twice on main: 2ae28eb + 480045a — both bare `git revert` invocations with no
commit-body explanation). The implementation was correct; the reverts appear
to be accidental-state-management, not a discovered bug. The live leak is
back on main as of 924c0fd (URL-embedded token in
scripts/check_consumer_runtime_drift.py:308 + scripts/check_platform_comm_contract.py:290).

What this commit changes
------------------------
* scripts/check_consumer_runtime_drift.py + scripts/check_platform_comm_contract.py
  - New `_git_clone_with_token()` helper writes a temporary 0o700 shell script
    that echoes 'x-access-token' for username prompts and shlex.quote(token)
    for password prompts. The script is unlinked in a `finally` clause.
  - clone_consumers / clone_repos call the helper, passing the token only via
    the GIT_ASKPASS env var — never in the git remote URL, never in
    subprocess argv, never in git diagnostics or logs.
  - Drop the now-unused `quote` import.
* tests/test_consumer_runtime_drift_guard.py
  - Restore Kimi's `test_clone_consumers_never_puts_token_in_argv` regression
    test that mocks subprocess.run and asserts the token does not appear in
    argv or the remote URL, and that GIT_ASKPASS is set in the env.

Pair this with the test_workflow_no_token_in_url.py gate (already on this
branch) so a future re-introduction of the URL-embedded pattern is red in CI
even if the helper itself is also reverted.

Why re-apply instead of leaving the leak live
---------------------------------------------
PM dispatched runtime#86 as a sibling auth-leak fix to runtime#162 (the
recent OAuth-leak fix). Both share the same family: a secret ends up in
a place where an attacker with log-read / process-inspection access can
read it. Leaving the live leak on main is the worse outcome — the
PR-with-gate approach (closed PR #166) was unmergeable precisely because
the gate correctly fires on the leak the live scripts re-introduce.

Diligence
---------
* Re-read Kimi's 061716f in full — implementation matches the standard
  GIT_ASKPASS pattern (subprocess.run with env override, 0o700 askpass,
  unlink in finally, shlex.quote on the token).
* No commit-body explanation on either revert (2ae28eb, 480045a) — both are
  bare `git revert` outputs.
* No comments on runtime#86 issue discussing why.
* No related PRs around June 8 that would have surfaced a real conflict.
* credential_helper work (runtime#104, fd2051d) sets `git config --global
  credential.helper` — different mechanism, no overlap with GIT_ASKPASS.
* Smoke-tested the helper locally: writes 0o700 script with the expected
  body, sets GIT_ASKPASS env, does not leak token/username into argv, and
  unlinks the file in the `finally` clause.

Tests
------
* tests/test_workflow_no_token_in_url.py — 4 new (gate; was already on branch).
* tests/test_consumer_runtime_drift_guard.py — 12 + 1 restored = 13 total
  (the +1 is Kimi's `test_clone_consumers_never_puts_token_in_argv`).
* tests/test_platform_comm_contract_guard.py — 6 existing.
* tests/test_llm_auth.py — 35 (unrelated, sanity).
* Total: 58 pass.

Independence from the red #3164 deployment surface
--------------------------------------------------
Pure scripts/test surface. No concierge / MCP / heartbeat / identity-gate /
operator-deployment touched. Safe to merge on the runtime-lane (CI-only gate).

Closes: runtime#86
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent-researcher approved these changes 2026-06-23 08:04:14 +00:00
agent-researcher left a comment
Member
Copy Link
Copy Source

APPROVE on 4e01b29336 (target=main).

Security/RCA review: the GIT_ASKPASS re-application closes the runtime#86 leak shape in the two audit clone paths. The clone URL is now token-free (https://host/molecule-ai/repo.git), the token is supplied only through a temporary 0700 askpass script as the password response, and subprocess argv / persisted remote URL no longer contain DISPATCH_TOKEN or x-access-token credentials. Error redaction still strips the raw token.

I do not see a reintroduced breakage explaining the prior accidental reverts: clone retry behavior is preserved, tokenless config still fails loudly, invalid Gitea URL validation remains, and the askpass env is scoped to the git clone subprocess. The only x-access-token command occurrence is the safe askpass username response.

Regression gate: robust enough for this class. It scans .gitea/workflows plus scripts/.py/.sh, asserts scan targets/non-vacuous candidates, fails on x-access-token: URL patterns, skips comment-only documentation, and pins the allowed askpass username occurrences to exactly the two clone helpers.

CI on this head is green: secret scan, lint, build, smoke-install, unit-tests, responsiveness-e2e. Local focused check passed: tests/test_consumer_runtime_drift_guard.py, tests/test_platform_comm_contract.py, tests/test_workflow_no_token_in_url.py (32/32).

APPROVE on 4e01b29336b865d30c7c7e657846f54e773ad7e9 (target=main). Security/RCA review: the GIT_ASKPASS re-application closes the runtime#86 leak shape in the two audit clone paths. The clone URL is now token-free (https://host/molecule-ai/repo.git), the token is supplied only through a temporary 0700 askpass script as the password response, and subprocess argv / persisted remote URL no longer contain DISPATCH_TOKEN or x-access-token credentials. Error redaction still strips the raw token. I do not see a reintroduced breakage explaining the prior accidental reverts: clone retry behavior is preserved, tokenless config still fails loudly, invalid Gitea URL validation remains, and the askpass env is scoped to the git clone subprocess. The only x-access-token command occurrence is the safe askpass username response. Regression gate: robust enough for this class. It scans .gitea/workflows plus scripts/*.py/*.sh, asserts scan targets/non-vacuous candidates, fails on x-access-token:<token-like> URL patterns, skips comment-only documentation, and pins the allowed askpass username occurrences to exactly the two clone helpers. CI on this head is green: secret scan, lint, build, smoke-install, unit-tests, responsiveness-e2e. Local focused check passed: tests/test_consumer_runtime_drift_guard.py, tests/test_platform_comm_contract.py, tests/test_workflow_no_token_in_url.py (32/32).
agent-reviewer-cr2 approved these changes 2026-06-23 08:04:22 +00:00
agent-reviewer-cr2 left a comment
Member
Copy Link
Copy Source

APPROVE on 4e01b29336 (target=main).

Security review:

  • Correctness: both clone helpers now build token-free HTTPS remote URLs and authenticate via GIT_ASKPASS, so the token is not present in subprocess argv and cannot be persisted into git remote URL configuration.
  • Robustness: retry/error paths keep using the token-free URL; stderr redaction still strips the raw token; the askpass temp file is removed in a finally block.
  • Security: no token bytes are introduced into warnings/log output, and the workflow/script regression gate scans the CI + script surfaces for x-access-token:<token/var> URL patterns with file:line failures. The positive-control tests make the gate non-vacuous and preserve only the safe askpass username occurrence.
  • Performance/readability: localized helper reapplication in the two clone scripts plus focused regression coverage; no broader runtime behavior change.

CI: own-head CI green (secret scan, lint, build, unit-tests, smoke-install, responsiveness-e2e).

APPROVE on 4e01b29336b865d30c7c7e657846f54e773ad7e9 (target=main). Security review: - Correctness: both clone helpers now build token-free HTTPS remote URLs and authenticate via GIT_ASKPASS, so the token is not present in subprocess argv and cannot be persisted into git remote URL configuration. - Robustness: retry/error paths keep using the token-free URL; stderr redaction still strips the raw token; the askpass temp file is removed in a finally block. - Security: no token bytes are introduced into warnings/log output, and the workflow/script regression gate scans the CI + script surfaces for x-access-token:<token/var> URL patterns with file:line failures. The positive-control tests make the gate non-vacuous and preserve only the safe askpass username occurrence. - Performance/readability: localized helper reapplication in the two clone scripts plus focused regression coverage; no broader runtime behavior change. CI: own-head CI green (secret scan, lint, build, unit-tests, smoke-install, responsiveness-e2e).
hongming merged commit d45e332ccb into main 2026-06-23 08:05:43 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
agent-researcher
agent-reviewer-cr2
Labels
Clear labels
triaged
PM triaged
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) molecule-code-reviewer molecule-runtime-release-bot (Molecule Runtime Release Bot) plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-ai-workspace-runtime#167
Delete Branch "fix/86-git-askpass-no-token-in-argv"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?