Summary
ghApi() builds a curl invocation as a single shell string with the token inlined, and runs it through execSync:
// scripts/fetch-apps.js:44-50
const res = execSync(
`curl -fsSL -H "Authorization: Bearer ${GITHUB_TOKEN}" -H "Accept: application/vnd.github+json" "${API_BASE}${path}"`,
{ encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }
);
Problems
- Token exposure. The full command line — token included — becomes the argv of a
sh -c process. It is readable by any process on the machine (ps, /proc/*/cmdline), and on CI it lands in whatever process/audit logging the runner does. It is also included verbatim in the Error thrown by execSync when the request fails, so a transient 404/5xx prints the token into build logs.
- Command injection.
path is built from apps.json values (repo, version) with no escaping — fetchRelease() does `/repos/${repo}/releases/tags/${version}`. A registry entry with a " or ` in repo/version breaks out of the quoted argument and executes arbitrary shell. apps.json is a reviewed file, but it is also the exact file an onboarding PR edits.
Note downloadAsset() (fetch-apps.js:78-85) already does the right thing — it uses spawnSync with an argv array, so the token never hits a shell.
Suggested fix
Drop the shell entirely. Node 24 has fetch, so ghApi() needs no subprocess at all:
const res = await fetch(`${API_BASE}${path}`, {
headers: { Authorization: `Bearer ${GITHUB_TOKEN}`, Accept: 'application/vnd.github+json' },
});
if (!res.ok) throw new Error(`GitHub API ${path} failed: ${res.status} ${res.statusText}`);
return res.json();
If a subprocess is preferred, use spawnSync('curl', [...args]) as downloadAsset does. Either way, validate repo against ^[\w.-]+/[\w.-]+$ and version against a tag-safe pattern when the registry is validated in build-vite.js, and make sure the token is never part of a thrown message.
Related: #56 (the same file's cp -r/tar shell-outs) and #44 (archive extraction hardening).
Summary
ghApi()builds acurlinvocation as a single shell string with the token inlined, and runs it throughexecSync:Problems
sh -cprocess. It is readable by any process on the machine (ps,/proc/*/cmdline), and on CI it lands in whatever process/audit logging the runner does. It is also included verbatim in theErrorthrown byexecSyncwhen the request fails, so a transient 404/5xx prints the token into build logs.pathis built fromapps.jsonvalues (repo,version) with no escaping —fetchRelease()does`/repos/${repo}/releases/tags/${version}`. A registry entry with a"or`inrepo/versionbreaks out of the quoted argument and executes arbitrary shell.apps.jsonis a reviewed file, but it is also the exact file an onboarding PR edits.Note
downloadAsset()(fetch-apps.js:78-85) already does the right thing — it usesspawnSyncwith an argv array, so the token never hits a shell.Suggested fix
Drop the shell entirely. Node 24 has
fetch, soghApi()needs no subprocess at all:If a subprocess is preferred, use
spawnSync('curl', [...args])asdownloadAssetdoes. Either way, validaterepoagainst^[\w.-]+/[\w.-]+$andversionagainst a tag-safe pattern when the registry is validated inbuild-vite.js, and make sure the token is never part of a thrown message.Related: #56 (the same file's
cp -r/tarshell-outs) and #44 (archive extraction hardening).