Skip to content

chore: sync core lib and CLAUDE.md from agent-core#27

Closed
avifenesh wants to merge 1 commit into
mainfrom
chore/sync-core-sync-docs-20260426-123555
Closed

chore: sync core lib and CLAUDE.md from agent-core#27
avifenesh wants to merge 1 commit into
mainfrom
chore/sync-core-sync-docs-20260426-123555

Conversation

@avifenesh

@avifenesh avifenesh commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

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-analyzer runtime downloader by verifying every GitHub release asset against a required .sha256 sidecar before any extraction, with an explicit local-dev-only skipChecksum escape 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-Archive with a PowerShell -File helper script that extracts via .NET ZipFile entry-by-entry using env-provided paths to avoid command-string parsing issues. Adds a comprehensive node:test suite covering checksum parsing/verification, path validation, scratch escape checks, and basic extraction flows.

Reviewed by Cursor Bugbot for commit a07e1ca. Configure here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread lib/binary/index.js
Comment on lines +303 to +318
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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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);
});

@avifenesh

Copy link
Copy Markdown
Contributor Author

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.

@avifenesh avifenesh closed this Apr 26, 2026
@avifenesh
avifenesh deleted the chore/sync-core-sync-docs-20260426-123555 branch April 26, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant