aws-samples / aws-samples/sample-autonomous-cloud-coding-agents
fix(agent): scope run_task git-identity + token env mutation to the call lifetime
- Lingua principale
- TypeScript
- Stelle
- 143
- Fork
- 46
- Merge medio
- 3g 10h
- PR unite (30g)
- 24
Descrizione
## Summary
Scope the process-global environment mutation in `run_task` (git identity + `gh` tokens) to the
lifetime of the call, so it cannot leak into a long-lived in-process caller.
## Background
[#623](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/623) fixed the
destructive `git config --global` clobber (#622) by switching to `GIT_AUTHOR_*` / `GIT_COMMITTER_*`
environment variables. That is the correct mechanism and is **strictly better** than the
predecessor — the old code persisted identity to `~/.gitconfig` and survived reboots; the new code
only persists for the lifetime of the Python process.
The variables are set via direct `os.environ[...]` writes in `run_task`:
https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/blob/5747dbdebd7af7b4976d388876d7963a939b7834/agent/src/pipeline.py#L824-L842
Because these are process-global, they persist after `run_task` returns for as long as the calling
process lives. This was raised as a non-blocking known-property note in the #623 review:
https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/623#discussion_r3606858056
## The residual edge (not a bug today)
No current caller triggers this — the AgentCore container runs `run_task` then exits, and the
dev-workstation path (#622's motivation) is a short-lived script that also exits. The latent edge is
a **long-lived in-process caller** that runs its own `git commit` *after* `run_task` in the same
process without setting identity: that commit would be silently attributed to `bgagent`.
The same block also writes `GITHUB_TOKEN` / `GH_TOKEN` to `os.environ` (`pipeline.py:835-836`), so
this is **token hygiene** as well as commit-attribution correctness — a lingering token in a
long-lived process is the more security-relevant leak of the two.
## Proposed fix (option B — scope, don't eliminate)
Keep the current mechanism (subprocesses — the Claude Code CLI spawn and the `post_hooks.py`
safety-net commit — inherit identity via `os.environ`, which is exactly why the env-var approach
works), but bound its lifetime with a `try/finally` restore. Because **every commit happens within
`run_task`'s scope**, restoring the prior environment at exit is safe — all commits are already done
by then.
A small context manager keeps it clean:
```python
from contextlib import contextmanager
@contextmanager
def _scoped_environ(**overrides: str):
"""Set env vars for the duration of the block, restoring prior values (or
deleting keys that were previously unset) on exit — even on exception."""
prior = {k: os.environ.get(k) for k in overrides}
os.environ.update(overrides)
try:
yield
finally:
for k, old in prior.items():
if old is None:
os.environ.pop(k, None)
else:
os.environ[k] = old
```
Then wrap the identity/token writes (and the downstream repo setup + agent run that depend on them)
in `with _scoped_environ(GIT_AUTHOR_NAME="bgagent", ...): ...` inside `run_task`.
### Why B and not the alternatives
- **Document-only:** acceptable but leaves the token-lingering property in place.
- **Eliminate via explicit `env=`:** threading an explicit env dict into every subprocess
(`setup_repo`, the CLI spawn, `post_hooks`) is fully correct but a cross-call-site refactor that
is easy to get wrong (the CLI spawn inherits `os.environ` today; `shell.py._clean_env()` strips
only `OTEL_*` / `PYTHONPATH`). Not worth it for a currently-unreachable edge.
- **B** neutralizes both the misattribution and the token-lingering edges in one localized change
with no call-site threading, and it is the production mirror of what the test already does — the
new regression test uses `monkeypatch.delenv(..., raising=False)` to restore env on teardown
(`agent/tests/test_pipeline.py:166`). B gives production the same lifecycle the test assumes.
## Acceptance criteria
- [ ] `run_task` restores (or deletes, if previously unset) `GIT_AUTHOR_NAME`, `GIT_AUTHOR_EMAIL`,
`GIT_COMMITTER_NAME`, `GIT_COMMITTER_EMAIL`, `GITHUB_TOKEN`, `GH_TOKEN`, `TASK_ID`,
`PROMPT_VERSION` on exit — on both the return path and the exception path.
- [ ] Subprocess commit paths (Claude Code CLI spawn + `post_hooks.py` safety-net commit) still
resolve identity to `bgagent ` (no regression to #622's fix).
- [ ] A regression test asserts that, after `run_task` returns, the six vars hold their
pre-call values (unset stays unset) in the calling process.
- [ ] Ruff clean; existing `test_pipeline.py` identity assertions still pass.
## References
- Follow-up to #623 (which fixed #622).
- Review note: https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/623#discussion_r3606858056
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Inizia in agent/src/pipeline.py, nelle scritture dell’ambiente di run_task intorno alle righe 824–842, poi esamina agent/tests/test_pipeline.py e il test esistente per il ripristino dell’ambiente intorno alla riga 166. Esegui i test della pipeline e Ruff; il lavoro è completato quando i valori dell’ambiente indicati vengono ripristinati nei percorsi di ritorno e di eccezione, mentre i commit di subprocess continuano a usare l’identità bgagent.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- git, github, python
- Ambito
- backend, devtools, security
- Tipo di issue
- Bug
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Stato di attività
- Tranquilla
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 76/100