feat: add volume mount for ~/.copilot/session-state to persist events.jsonl#1469
feat: add volume mount for ~/.copilot/session-state to persist events.jsonl#1469
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.72% | 82.75% | 📈 +0.03% |
| Statements | 82.38% | 82.42% | 📈 +0.04% |
| Functions | 81.50% | 81.50% | ➡️ +0.00% |
| Branches | 76.19% | 76.16% | 📉 -0.03% |
📁 Per-file Coverage Changes (1 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
86.0% → 86.1% (+0.08%) | 85.5% → 85.6% (+0.08%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
This comment has been minimized.
This comment has been minimized.
* Initial plan * fix: update handlebars and brace-expansion to fix npm audit vulnerabilities Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/41a2d615-b628-46d7-bf1f-2e21f94f7f5b Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.72% | 82.77% | 📈 +0.05% |
| Statements | 82.38% | 82.43% | 📈 +0.05% |
| Functions | 81.50% | 81.22% | 📉 -0.28% |
| Branches | 76.19% | 76.00% | 📉 -0.19% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
61.2% → 61.1% (-0.10%) | 61.6% → 61.5% (-0.10%) |
src/docker-manager.ts |
86.0% → 86.2% (+0.23%) | 85.5% → 85.7% (+0.24%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
Smoke Test Results — PASS
|
Smoke Test Results — PASS
Overall: PASS —
|
Chroot Version Comparison Results
Overall: Not all tests passed. Python and Node.js versions differ between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
|
Smoke test (run 23725710566)
|
|
@copilot are there any potential security issues with persisting events.jsonl? for example, could a prompt-injected agent modify the content events.jsonl to exfiltrate sensitive information? |
The short answer is: this mount doesn't create a new exfiltration path, but here's the full analysis: Network exfiltration via Content tampering is a valid (but pre-existing) concern — the
The practical mitigations already in place: the file never leaves the machine without an explicit network request through an allowed domain, and the host path is under If there's a concern about a specific downstream consumer being vulnerable to log injection from this file, that's worth hardening at the consumer — but it's outside the scope of this mount. |
There was a problem hiding this comment.
Pull request overview
Adds persistence for Copilot CLI’s structured session-state (events.jsonl) by introducing a new host-mounted directory and documenting where it’s preserved after runs, so dashboards/debugging can rely on the JSONL artifact.
Changes:
- Add a new Docker volume mount for
~/.copilot/session-statebacked by${workDir}/agent-session-state. - Create
${workDir}/agent-session-stateduring pre-run config setup and preserve it to a timestamped/tmpdirectory on cleanup. - Document the new preserved session-state location in
docs/usage.md.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/docker-manager.ts | Adds the session-state volume mount, creates the session-state workDir, and preserves it on cleanup. |
| package-lock.json | Updates transitive dependency versions (brace-expansion, handlebars). |
| docs/usage.md | Documents preserved session-state output and updates volume mapping explanation. |
Comments suppressed due to low confidence (2)
src/docker-manager.ts:1558
agent-session-stateis created without setting ownership/permissions. When AWF is run viasudo, this directory will likely be root-owned; but in chroot mode the command is executed as the mapped host user (non-root), so Copilot may be unable to createevents.jsonlin this bind mount. Set ownership to the host UID/GID (same as the chroot-home setup) and choose an explicit mode that matches your intended readability.
// Create agent session-state directory for persistence (events.jsonl written by Copilot CLI)
const agentSessionStateDir = path.join(config.workDir, 'agent-session-state');
if (!fs.existsSync(agentSessionStateDir)) {
fs.mkdirSync(agentSessionStateDir, { recursive: true });
}
logger.debug(`Agent session-state directory created at: ${agentSessionStateDir}`);
src/docker-manager.ts:2126
- Preservation uses
fs.renameSyncto moveagent-session-stateintoos.tmpdir().renamefails withEXDEVwhenworkDirand/tmpare on different filesystems (e.g., custom workDir on another mount), which would silently skip preservation. Add a copy+remove fallback (or handle EXDEV explicitly) so the feature works reliably across filesystems.
// Preserve agent session-state before cleanup (contains events.jsonl from Copilot CLI)
const agentSessionStateDir = path.join(workDir, 'agent-session-state');
const agentSessionStateDestination = path.join(os.tmpdir(), `awf-agent-session-state-${timestamp}`);
if (fs.existsSync(agentSessionStateDir) && fs.readdirSync(agentSessionStateDir).length > 0) {
try {
fs.renameSync(agentSessionStateDir, agentSessionStateDestination);
logger.info(`Agent session state preserved at: ${agentSessionStateDestination}`);
} catch (error) {
logger.debug('Could not preserve agent session state:', error);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Mount agent logs directory to workDir for persistence | ||
| `${config.workDir}/agent-logs:${effectiveHome}/.copilot/logs:rw`, | ||
| // Mount agent session-state directory to workDir for persistence (events.jsonl) | ||
| `${config.workDir}/agent-session-state:${effectiveHome}/.copilot/session-state:rw`, |
There was a problem hiding this comment.
The session-state volume is mounted to ${effectiveHome}/.copilot/session-state, but when AWF runs in chroot mode the user command writes to paths under /host (see entrypoint.sh chroot execution). As a result, ~/.copilot/session-state inside the chroot won’t see this mount and events.jsonl may still be discarded. Consider mounting the same host dir to /host${effectiveHome}/.copilot/session-state (and/or ensuring Copilot is configured to write to the non-chroot path).
This issue also appears in the following locations of the same file:
- line 1553
- line 2116
| `${config.workDir}/agent-session-state:${effectiveHome}/.copilot/session-state:rw`, | |
| `${config.workDir}/agent-session-state:${effectiveHome}/.copilot/session-state:rw`, | |
| // When running in chroot mode, the user's home is accessed under /host, | |
| // so also mount the same session-state directory to the chrooted home path. | |
| `${config.workDir}/agent-session-state:/host${effectiveHome}/.copilot/session-state:rw`, |
| // Create agent session-state directory for persistence (events.jsonl written by Copilot CLI) | ||
| const agentSessionStateDir = path.join(config.workDir, 'agent-session-state'); | ||
| if (!fs.existsSync(agentSessionStateDir)) { | ||
| fs.mkdirSync(agentSessionStateDir, { recursive: true }); | ||
| } | ||
| logger.debug(`Agent session-state directory created at: ${agentSessionStateDir}`); |
There was a problem hiding this comment.
New behavior (creating agent-session-state, preserving it on cleanup, and the new volume mount) isn’t covered by tests. src/docker-manager.test.ts already has coverage for the analogous agent-logs preservation; please add similar assertions for session-state so regressions are caught.
| **How it works:** | ||
| - GitHub Copilot CLI writes to `~/.copilot/logs/`, Squid writes to `/var/log/squid/` | ||
| - Volume mounts map these to `${workDir}/agent-logs/` and `${workDir}/squid-logs/` | ||
| - Before cleanup, logs are automatically moved to `/tmp/awf-agent-logs-<timestamp>/` and `/tmp/squid-logs-<timestamp>/` (if they exist) | ||
| - GitHub Copilot CLI writes to `~/.copilot/logs/` and `~/.copilot/session-state/`; Squid writes to `/var/log/squid/` | ||
| - Volume mounts map container paths to: | ||
| - `${workDir}/agent-logs/` → `~/.copilot/logs/` | ||
| - `${workDir}/agent-session-state/` → `~/.copilot/session-state/` | ||
| - `${workDir}/squid-logs/` → `/var/log/squid/` | ||
| - Before cleanup, non-empty directories are automatically moved to timestamped `/tmp` paths: | ||
| - `/tmp/awf-agent-logs-<timestamp>/` | ||
| - `/tmp/awf-agent-session-state-<timestamp>/` | ||
| - `/tmp/squid-logs-<timestamp>/` |
There was a problem hiding this comment.
The “How it works” section describes mounts for ~/.copilot/session-state, but in AWF chroot mode the agent command runs inside /host (entrypoint.sh), so the effective path being written is under /host$HOME/.... Once the mount points are corrected, please update this section to reflect the chroot behavior (or clarify that the mount targets both chroot and non-chroot paths).
|
Smoke Test Results — PASS
|
Copilot CLI writes
events.jsonlto~/.copilot/session-state/inside the container, but AWF had no volume mount for that path — the file was discarded when the container stopped, leavingagent_outputsartifacts without the structured conversation JSONL needed for dashboards and debugging.Changes
generateDockerCompose): addsworkDir/agent-session-state → ~/.copilot/session-statealongside the existingagent-logsmountwriteConfigs): createsworkDir/agent-session-state/before container startagent-session-state/to/tmp/awf-agent-session-state-<timestamp>after the run, matching theagent-logspatterndocs/usage.md): documents the new preserved path and updates the "How it works" volume mapping listAfter a Copilot CLI run,
events.jsonlis available at:⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.