chore: sync core lib and CLAUDE.md from agent-core#27
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces significant security hardening for the binary downloader and extractor. Key improvements include SHA-256 checksum verification via sidecar files, strict archive entry validation to prevent path traversal attacks, and the use of isolated scratch directories for extraction. On Windows, the implementation now uses a custom PowerShell script with .NET's ZipFile API for safer path handling. Comprehensive tests have been added to verify these security measures. A review comment correctly identifies a potential stability issue where unhandled EPIPE errors during stdin writes to the tar process could lead to crashes.
| const tar = cp.spawn('tar', ['-tz'], { stdio: ['pipe', 'pipe', 'pipe'] }); | ||
| let stdout = ''; | ||
| let stderr = ''; | ||
| tar.stdout.on('data', function(d) { stdout += d; }); | ||
| tar.stderr.on('data', function(d) { stderr += d; }); | ||
| tar.stdin.write(buf); | ||
| tar.stdin.end(); | ||
| tar.on('error', reject); | ||
| tar.on('close', function(code) { | ||
| if (code !== 0) { | ||
| reject(new Error('tar extraction failed (code ' + code + '): ' + stderr)); | ||
| } else { | ||
| resolve(); | ||
| reject(new Error('tar -tz listing failed (code ' + code + '): ' + stderr)); | ||
| return; | ||
| } | ||
| const entries = stdout.split(/\r?\n/).filter(function(l) { return l.length > 0; }); | ||
| resolve(entries); | ||
| }); | ||
| tar.on('error', reject); | ||
| tar.stdin.write(buf); | ||
| tar.stdin.end(); |
There was a problem hiding this comment.
The tar.stdin.write(buf) call in listTarGzEntries (and similarly in extractTarGzToScratch) does not handle potential EPIPE errors. If the tar process exits unexpectedly or early (e.g., due to a corrupted buffer or an unsupported archive format), writing to its stdin will throw an unhandled EPIPE error, which can crash the Node.js process. It is recommended to add an error listener to tar.stdin to catch these cases, for example:
tar.stdin.on('error', function(err) {
if (err.code !== 'EPIPE') reject(err);
});|
Closing: the sync workflow that generated this PR mirrored agent-core over consumer repos and deleted downstream-only files. See agent-sh/agent-core#14 for the additive-sync fix. A fresh sync PR will be generated automatically after merge. |
Automated sync of lib/ and CLAUDE.md from agent-core.
Note
Medium Risk
Changes the binary download/install path to require SHA-256 sidecar verification and to extract archives into validated scratch dirs, which could break installs if release assets/sidecars or archive layouts differ across platforms (especially Windows). Security impact is positive but the new extraction and PowerShell logic is complex and platform-dependent.
Overview
Hardens the
agent-analyzerruntime downloader by verifying every GitHub release asset against a required.sha256sidecar before any extraction, with an explicit local-dev-onlyskipChecksumescape hatch.Reworks extraction to use an isolated scratch directory: tar/zip entries are pre-validated to block absolute/UNC/drive-letter paths and
..traversal, extracted content is post-walked to reject symlinks/escapes, and only the expected binary is copied into~/.agent-sh/bin(everything else is discarded).On Windows, replaces
Expand-Archivewith a PowerShell-Filehelper script that extracts via .NETZipFileentry-by-entry using env-provided paths to avoid command-string parsing issues. Adds a comprehensivenode:testsuite covering checksum parsing/verification, path validation, scratch escape checks, and basic extraction flows.Reviewed by Cursor Bugbot for commit a07e1ca. Configure here.