aws-samples / aws-samples/sample-autonomous-cloud-coding-agents
fix(agent): scope run_task git-identity + token env mutation to the call lifetime
- Dominant language
- TypeScript
- Stars
- 143
- Forks
- 46
- Avg merge
- 3d 9h
- Merged PRs (30d)
- 20
Description
## 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
Contributor guide
Research direction
Start in agent/src/pipeline.py at run_task’s environment writes around lines 824–842, then inspect agent/tests/test_pipeline.py and the existing environment restoration test around line 166. Run the pipeline tests and Ruff; done means the listed environment values are restored on return and exception paths while subprocess commits still use the bgagent identity.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, github, python
- Domain
- backend, devtools, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100