A coworker opened PR #1639 with a GitProxy plugin that clones the upstream repository on pull and scans the tree before the fetch is forwarded. The idea is supply-chain defence: stop a developer pulling something malicious. Two details in that clone-and-scan path are worth writing down even if you never touch GitProxy, because they show up in any tool that shells out to git and any scanner that would rather warn than block.
The credential is on the command line
The clone is constructed like this:
cloneArgs.push('-c', `http.extraHeader=Authorization: ${auth}`);That string becomes an argument of a spawned git process. On Linux, every local user can read another process’s arguments from /proc/<pid>/cmdline, and ps is the same information with nicer formatting. CWE-214 is this pattern: putting a secret in the invocation of a process that is visible to other users on the machine.
The comment next to the code said the header is never logged. That is not the same as never visible. simple-git’s error objects routinely include the full command string, and the catch block did this:
step.log(`supply-chain pull scan error: ${e?.message ?? e}`);The step lands in GitProxy’s audit database and the dashboard. A clone that fails for any reason can persist the developer’s token in a place that is meant to be read later.
Git already has a way to pass configuration without putting the value in argv. git --config-env=<name>=<envvar> reads the value from the environment, which is not in /proc/<pid>/cmdline. A 0600 temp config file, pointed at with -c include.path=... or GIT_CONFIG_GLOBAL, is the other common shape. Either one, plus scrubbing the caught error before it is written to the step, closes both the argv leak and the log leak.
http.extraHeader is also not host-scoped. If the upstream responds with a redirect, the header goes with it. That is a third reason not to put a bearer token there, separate from whether argv is visible.
The scanner fails open
Any exception in clone, auth, ls-tree, or analysis is swallowed, and the action returns clean. As a default for a warn-mode annotator that is a reasonable product choice: a scanner that cannot run should not block every fetch. The moment someone sets pull.failOn: "critical", though, they believe they have a control, and an attacker who can make the scanner fail gets a silent pass. Fill /tmp, time the clone out, push a repo large enough to blow the process, and the pull goes through.
CWE-636 is this shape: not failing securely. OWASP’s Fail securely write-up is the same advice in different words. A control that is on a fail-open path is not a control, it is a best-effort warning, and the configuration should say so in those words. If failOn is set, a scanner error has to fail closed: reject the pull, or at least refuse to claim the scan ran.
There is no timeout, no depth cap, and no concurrency limit on those clones either, and the pull chain runs on both /info/refs and /git-upload-pack, so a single git clone from a user costs two full clones through the proxy unless something deduplicates. On a monorepo behind a shared GitProxy that is a self-inflicted denial of service sitting next to the fail-open path.
The clone is also the wrong clone
The PR discloses this one plainly, and it is the reason I would not put the scanner in a plugin at the current chain position at all. Pull scanning as written is HTTPS-only and scans the default branch rather than the requested ref, so the content inspected can differ from the content delivered. Put the payload on a non-default branch and the scan never sees it. I went through why the pull chain cannot see response bytes in how to write a GitProxy plugin, and why a second clone cannot close that gap.
What to do instead of either of these
If you need to give git a credential, put it in the environment or in a mode-0600 config file, never in argv, and never in a string you later interpolate into an audit log. If you need a scanner to be a control rather than a warning, fail closed when the scanner cannot run, cap what it is allowed to consume, and scan the bytes the user actually receives rather than a guess at them. The last of those is not a plugin-shaped change; it belongs in the proxy’s response path, which is the work I would rather do next than ship a clone-inside-the-plugin that fails open with a token on ps.