test: automated scanning for large changesets

[jescalada]

Testing whether AI review can handle large changesets, and expected cost

[github-actions]

Thanks for the contribution!

The PR description does not clearly explain what changed or why. "Testing whether AI review can handle large changesets, and expected cost" describes an experiment rather than the purpose of the code changes themselves. It would help reviewers to understand what the tests cover, what behavior they verify, and why they are being added now.

Please link this PR to an existing issue, or create a new one if none exists. From CONTRIBUTING.md:

"Check for existing issues: Search open issues before starting work. If none exists, create one describing the change."

Once an issue exists, add a line such as Closes #N or Related to #N to the PR description so the connection is tracked.

[github-actions]

Automated Security Review

Summary

The new supply-chain plugin is mostly self-contained static analysis, but the pull/clone scanner forwards the client's Authorization header into a git clone of a URL that is only loosely validated (including plain http://), and untrusted manifest content is echoed into git protocol packets and into the clone command line without normalisation. No hardcoded secrets, SQL/shell injection, or unsafe deserialization were found.

Findings

plugins/git-proxy-plugin-supply-chain/index.js
Credential exposure / insecure transport
In pullExec, the client's Authorization header is re-sent to the upstream URL via http.extraHeader, and the guard explicitly permits plain HTTP (/^https?:/i). On an http:// upstream the bearer/basic credential is transmitted in cleartext. http.extraHeader is also applied to every request git makes for that clone, so if the upstream issues a redirect to another host, the credential follows it. The URL itself is only validated with url.includes('NOT-FOUND').

const auth = req?.headers?.authorization;
const cloneArgs = ['clone', '--depth', '1', '--single-branch', '--quiet'];
if (auth && /^https?:/i.test(url)) {
  cloneArgs.push('-c', `http.extraHeader=Authorization: ${auth}`);
}
cloneArgs.push(url, cloneDir);
await simpleGit().raw(cloneArgs);

Recommended fix: parse url with new URL() and require protocol === 'https:' before attaching the header; additionally set -c http.followRedirects=false (or credential.helper= and -c protocol.ext.allow=never) so the credential cannot leak to a redirect target.

plugins/git-proxy-plugin-supply-chain/index.js
Missing input validation on the clone target (argument injection / unsafe transport)
url is taken from the action and appended directly as a positional argument to git clone. If a value ever begins with - it is interpreted as a git option, and non-http(s) transports such as ext:: or file:// would be accepted because the scheme is never asserted before the clone (only before the auth header is added). simple-git argv passing avoids a shell, but not git's own option/transport parsing.

if (!url || url.includes('NOT-FOUND')) { ... }
...
cloneArgs.push(url, cloneDir);
await simpleGit().raw(cloneArgs);

Recommended fix: validate the scheme with new URL(url) and reject anything other than https: (and optionally http: with no credentials); pass -- before the positional URL, e.g. cloneArgs.push('--', url, cloneDir).

plugins/git-proxy-plugin-supply-chain/lib/findings.js, plugins/git-proxy-plugin-supply-chain/lib/ecosystems/*.js, src/proxy/routes/index.ts
Untrusted content echoed to terminal / protocol packet without sanitisation
Finding title/detail embed raw repository content (package names, script bodies, URLs). truncate() collapses whitespace only, so control characters such as ESC (\x1b) survive and are written into the ERR pkt-line delivered to the developer's terminal via handleErrPacket, enabling terminal escape-sequence injection (screen rewriting, hidden text) from a malicious repository. The same strings are also stored in the step content that the dashboard renders.

const s = String(str).replace(/\s+/g, ' ').trim();

const errorBody = `ERR ${message}\n`;

Recommended fix: strip or escape non-printable/control characters (e.g. replace(/[\x00-\x1f\x7f]/g, '?')) in truncate() before the value reaches renderFindings/handleErrPacket.

plugins/git-proxy-plugin-supply-chain/lib/typosquat.js
Unbounded input to an O(n·m) algorithm (denial of service)
nearestInSet runs levenshtein between an attacker-controlled dependency name (arbitrary length, taken from a pushed package.json/requirements.txt) and every entry of the ~180-name popular list, once per dependency. A manifest with many very long names can consume large amounts of CPU inside the push chain.

for (const pop of popularSet) {
  const d = levenshtein(bare, pop);

Recommended fix: skip names longer than a small bound (e.g. bare.length > 64) and cap the number of dependencies scanned per manifest; the same applies to nearestPopularGo in lib/ecosystems/go.js.

plugins/git-proxy-plugin-supply-chain/index.js
Security control can be bypassed (scanned content differs from delivered content)
The pull scanner shallow-clones and scans only the default branch, while the client may fetch any ref. A repository can therefore keep a clean default branch and serve poisoned content on another branch/tag, passing the pull.failOn block. This is acknowledged in the docs, but it means the blocking mode does not provide the guarantee its error message implies.

const cloneArgs = ['clone', '--depth', '1', '--single-branch', '--quiet'];

Recommended fix: derive the requested ref from the upload-pack request and scan that ref, or make the block message explicit that only the default branch was inspected so operators do not over-trust the control.

Note

No instructions embedded in the PR description or diff attempted to influence this review.

---

Coverage: 33 of 33 changed files were reviewed.

Disclaimer: This review is AI-generated and covers only what is listed above. Please validate the findings before acting on them.

_{Reviewed by claude-opus-5. Re-run by commenting /security-review on this PR.}

[jescalada]

/security-review

[jescalada]

/security-review