aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

fix(agent): scope run_task git-identity + token env mutation to the call lifetime

Offen
#635 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
agent-runtime enhancement P2
Vorherrschende Sprache
TypeScript
Sterne
143
Forks
46
Ø Merge
3 T. 10 Std.
Gemergte PRs (30 T.)
24

Beschreibung

## 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

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne in agent/src/pipeline.py bei den Umgebungs-Schreibvorgängen von run_task um die Zeilen 824–842 und prüfe anschließend agent/tests/test_pipeline.py sowie den bestehenden Test zur Wiederherstellung der Umgebung um Zeile 166. Führe die Pipeline-Tests und Ruff aus; als erledigt gilt, dass die aufgeführten Umgebungswerte bei Rückgabe- und Ausnahme-Pfaden wiederhergestellt werden, während Commits von subprocess weiterhin die Identität bgagent verwenden.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
git, github, python
Bereich
backend, devtools, security
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
76/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.