Summary
_patch_env_github_pat replaces the first GITHUB_PAT= line only (count=1), while the agent's own .env parser is last-wins. An .env carrying two GITHUB_PAT lines therefore ends a rotation with the new token on line 1 and the revoked token still on line 2 — and the agent reads the revoked one. The rotation reports the agent as updated.
Found by /edge-cases round-trip analysis of #1979 (issue #1967). Same failure class that issue exists to fix — "the rotation reports success and the agent keeps using the old token" — via a different route.
Mechanism
src/backend/services/github_pat_propagation_service.py:
line_re = re.compile(rf'(?m)^[ \t]*{key}=.*$')
if line_re.search(out):
out = line_re.sub(new_line, out, count=1) # first match onlyAgent side (agent_server/services/execution_env.parse_env_file, and the credential-inject mirror before it) walks every line and assigns parsed[key] = value — the last occurrence wins.
Reproduction
Executed against the merged code, not inferred:
in: GITHUB_PAT="old-token"
FOO=1
GITHUB_PAT="old-token"
out: GITHUB_PAT="ghp_NEWTOKEN"
FOO=1
GITHUB_PAT="old-token" <-- survives
GH_TOKEN="ghp_NEWTOKEN"
GITHUB_TOKEN="ghp_NEWTOKEN"
agent reads GITHUB_PAT = 'old-token'
tests/unit/test_pat_propagation_properties.py::TestKnownGaps::test_a_duplicated_pat_line_still_rotates (xfail(strict=True)).
The same file states the general contract as a Hypothesis property — after a rotation the agent reads back exactly the new token under all three key names — which holds for every realistic single-line .env; this is the input where it does not.
How a duplicate line gets there
Not from this function: when the key is absent it appends, and when present it substitutes. The routes are:
So: uncommon, but silent and credential-shaped when it happens, and the fix is small.
Severity
P3 rather than P2 because it needs a pre-existing duplicate line. Raising it if that turns out to be reachable from a supported flow.
The old token also survives as text in the file regardless of which line wins, so anything sourcing or grepping .env still finds a revoked credential. The property test_no_token_is_left_behind_anywhere_in_the_file covers that direction.
Suggested fix
Drop count=1 (replace every occurrence), or normalize to a single line per key. Either makes the last-wins reader agree with the writer. A test asserting what the agent reads back, rather than what the file contains, is the one that would have caught this.
Acceptance criteria
Related
Summary
_patch_env_github_patreplaces the firstGITHUB_PAT=line only (count=1), while the agent's own.envparser is last-wins. An.envcarrying twoGITHUB_PATlines therefore ends a rotation with the new token on line 1 and the revoked token still on line 2 — and the agent reads the revoked one. The rotation reports the agent asupdated.Found by
/edge-casesround-trip analysis of #1979 (issue #1967). Same failure class that issue exists to fix — "the rotation reports success and the agent keeps using the old token" — via a different route.Mechanism
src/backend/services/github_pat_propagation_service.py:Agent side (
agent_server/services/execution_env.parse_env_file, and the credential-inject mirror before it) walks every line and assignsparsed[key] = value— the last occurrence wins.Reproduction
Executed against the merged code, not inferred:
tests/unit/test_pat_propagation_properties.py::TestKnownGaps::test_a_duplicated_pat_line_still_rotates(xfail(strict=True)).The same file states the general contract as a Hypothesis property — after a rotation the agent reads back exactly the new token under all three key names — which holds for every realistic single-line
.env; this is the input where it does not.How a duplicate line gets there
Not from this function: when the key is absent it appends, and when present it substitutes. The routes are:
.env(agents can write that file — see bug: credential injects mirror.envinto the agent-server long-lived process env — a removed key keeps reaching every execution, invisible to /proc and docker exec #1999),docker exec,.env.So: uncommon, but silent and credential-shaped when it happens, and the fix is small.
Severity
P3 rather than P2 because it needs a pre-existing duplicate line. Raising it if that turns out to be reachable from a supported flow.
The old token also survives as text in the file regardless of which line wins, so anything sourcing or grepping
.envstill finds a revoked credential. The propertytest_no_token_is_left_behind_anywhere_in_the_filecovers that direction.Suggested fix
Drop
count=1(replace every occurrence), or normalize to a single line per key. Either makes the last-wins reader agree with the writer. A test asserting what the agent reads back, rather than what the file contains, is the one that would have caught this.Acceptance criteria
.envcontained duplicate linesRelated