chore: sync core lib and CLAUDE.md from agent-core - #22
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new binary resolver for agent-analyzer, enabling automated downloading, checksum verification, and secure extraction across different platforms. It also refactors the repo-map functionality to use this new binary resolver, including modules for caching, format conversion, and staleness checking. Feedback was provided regarding the network request logic, specifically suggesting the addition of a timeout to https.get and improving the robustness of redirect handling to support relative URLs and handle missing headers.
| }; | ||
| if (ghToken) headers['Authorization'] = 'Bearer ' + ghToken; | ||
|
|
||
| https.get(reqUrl, { headers: headers }, function(res) { |
There was a problem hiding this comment.
The https.get call lacks a timeout. In environments with unstable network connections, this request could hang indefinitely, blocking the execution of the tool. It is recommended to set a socket timeout (e.g., 30 seconds) and handle the timeout event by destroying the request and rejecting the promise.
| if (sc === 301 || sc === 302 || sc === 307 || sc === 308) { | ||
| res.resume(); | ||
| request(res.headers.location, redirectCount + 1); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The redirect handling logic is fragile. It assumes res.headers.location is always present and that it is an absolute URL. If the server returns a relative URL in the Location header, https.get will fail. Additionally, if the header is missing, it will attempt to call request(undefined, ...) which will throw a type error.
| if (sc === 301 || sc === 302 || sc === 307 || sc === 308) { | |
| res.resume(); | |
| request(res.headers.location, redirectCount + 1); | |
| return; | |
| } | |
| if (sc === 301 || sc === 302 || sc === 307 || sc === 308) { | |
| res.resume(); | |
| const location = res.headers.location; | |
| if (!location) { | |
| reject(new Error('Redirect response missing Location header from ' + reqUrl)); | |
| return; | |
| } | |
| // Resolve relative URLs against the current request URL | |
| const nextUrl = new URL(location, reqUrl).toString(); | |
| request(nextUrl, redirectCount + 1); | |
| return; | |
| } |
|
Superseded by fresh sync after agent-core upstreamed missing files (workflow-state.js full version + lib/repo-intel/queries.js). Close older PR to avoid conflicts. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e790e92. Configure here.
| const sc = res.statusCode; | ||
| if (sc === 301 || sc === 302 || sc === 307 || sc === 308) { | ||
| res.resume(); | ||
| request(res.headers.location, redirectCount + 1); |
There was a problem hiding this comment.
Auth header leaked on cross-origin redirects causes 403s
High Severity
The downloadToBuffer function unconditionally sends the Authorization: Bearer header when following redirects. This leaks credentials to cross-origin CDN domains, such as those used by GitHub for release assets, and can result in 403 errors. Standard HTTP practice recommends stripping auth headers on cross-origin redirects.
Reviewed by Cursor Bugbot for commit e790e92. Configure here.
| return { | ||
| version: '2.0', | ||
| generated: intel.generated || new Date().toISOString(), | ||
| git: intel.git ? { commit: intel.git.analyzedUpTo } : undefined, |
There was a problem hiding this comment.
Converter omits branch, disabling staleness branch-change detection
Medium Severity
The convertIntelToRepoMap function only populates the git.commit field, leaving git.branch undefined. This prevents the branch-change staleness detection logic from ever triggering, making it effectively dead code.
Reviewed by Cursor Bugbot for commit e790e92. Configure here.
| */ | ||
| async function ensureRepoMap(options = {}) { | ||
| const { cwd = process.cwd(), askUser } = options; | ||
| const repoMap = getRepoIntel(); |
There was a problem hiding this comment.
Stale ast-grep prompts after switching to auto-download backend
Medium Severity
The getRepoMap refactor causes ensureRepoMap to incorrectly prompt users to "install ast-grep" when the agent-analyzer auto-download fails. This user-facing message and related fallback reasons are misaligned with the new auto-download mechanism.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e790e92. Configure here.
1 participant
Automated sync of lib/ and CLAUDE.md from agent-core.
Note
High Risk
Introduces runtime downloading/extraction and execution of an external binary, including new archive parsing/extraction paths (tar/PowerShell zip) that are security-sensitive and platform-dependent.
Overview
Adds a new
lib/binarymodule that lazily downloads theagent-analyzerGitHub release at runtime, enforces a minimum version, and verifies each asset via a.sha256sidecar before extracting only the expected binary into~/.agent-sh/bin/.Hardens extraction by validating archive entry paths (no absolute/UNC/drive-letter/
..traversal), extracting into isolated scratch dirs, rejecting symlinks, and using a PowerShell-Filehelper on Windows to avoid command-string re-parsing.Introduces a new
lib/repo-mapsubsystem that runsagent-analyzer repo-intel init/update, convertsrepo-intel.jsoninto cachedrepo-map.json, and adds staleness checks; existing collectors are rewired to use the new localbinary/repo-mapmodules (including exportingrepoMapandbinaryfromlib/index.jsand simplifyingdocs-patterns/gitcollectors).Reviewed by Cursor Bugbot for commit e790e92. Configure here.