diff --git a/changelog.d/62.md b/changelog.d/62.md new file mode 100644 index 0000000..9adee0d --- /dev/null +++ b/changelog.d/62.md @@ -0,0 +1 @@ +- Keep Debian registry tokens out of curl process arguments and clean upload credentials and responses on every exit. (#62). diff --git a/scripts/publish-deb.sh b/scripts/publish-deb.sh index f3b7e66..7feff43 100755 --- a/scripts/publish-deb.sh +++ b/scripts/publish-deb.sh @@ -40,11 +40,24 @@ EOF exit 1 fi +if [ -n "${RUNNER_TEMP:-}" ]; then + TMP="$(mktemp -d "$RUNNER_TEMP/stoke-publish.XXXXXX")" +else + TMP="$(mktemp -d)" +fi +trap 'rm -rf "$TMP"' EXIT + +HEADER_FILE="$TMP/authorization-header" +RESPONSE_FILE="$TMP/response" +umask 077 +printf 'Authorization: token %s\n' "$TOKEN" >"$HEADER_FILE" +chmod 0600 "$HEADER_FILE" + URL="$FORGE_URL/api/packages/$OWNER/debian/pool/$DISTRIBUTION/$COMPONENT/upload" echo "Uploading $(basename "$DEB") to $URL" -STATUS="$(curl -sS -o /tmp/stoke-publish-response.$$ -w '%{http_code}' \ - -X PUT -H "Authorization: token $TOKEN" \ +STATUS="$(curl -sS -o "$RESPONSE_FILE" -w '%{http_code}' \ + -X PUT -H @"$HEADER_FILE" \ --upload-file "$DEB" "$URL")" case "$STATUS" in @@ -52,9 +65,7 @@ case "$STATUS" in 409) echo "Already published (409): this exact version already exists in the registry." ;; *) echo "error: upload failed with HTTP $STATUS" >&2 - cat /tmp/stoke-publish-response.$$ >&2 || true - rm -f /tmp/stoke-publish-response.$$ + cat "$RESPONSE_FILE" >&2 || true exit 1 ;; esac -rm -f /tmp/stoke-publish-response.$$ diff --git a/test/publish-deb.test.js b/test/publish-deb.test.js index 04f84f2..b1c4561 100644 --- a/test/publish-deb.test.js +++ b/test/publish-deb.test.js @@ -7,51 +7,152 @@ const path = require('node:path'); const ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'publish-deb.sh'); -const TOKEN = 'deb-token-that-must-not-appear-in-output'; +const TOKEN = 'deb-token-that-must-not-enter-argv'; +const UPLOAD_LINE = 'Uploading stoke_2.0.0_all.deb to https://forge.example.test/api/packages/heavy-duty/debian/pool/stable/main/upload\n'; -function runScenario({ token = '' } = {}) { +function runScenario({ token = '', httpStatus = 201, responseBody = '', curlStatus = 0 } = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-publish-deb-test-')); + const home = path.join(dir, 'home'); + const bin = path.join(dir, 'bin'); + const runnerTemp = path.join(dir, 'runner-temp'); + const log = path.join(dir, 'curl.json'); + const deb = path.join(dir, 'stoke_2.0.0_all.deb'); + const legacyBefore = new Set(fs.readdirSync(os.tmpdir()).filter((name) => name.startsWith('stoke-publish-response.'))); + let call = null; + let result; + try { - const home = path.join(dir, 'home'); - const bin = path.join(dir, 'bin'); - const deb = path.join(dir, 'stoke_2.0.0_all.deb'); fs.mkdirSync(home); fs.mkdirSync(bin); + fs.mkdirSync(runnerTemp); fs.writeFileSync(deb, 'package'); - fs.writeFileSync(path.join(bin, 'curl'), '#!/usr/bin/env bash\nprintf 201\n'); + fs.writeFileSync(path.join(bin, 'curl'), `#!/usr/bin/env node +const fs = require('node:fs'); +const path = require('node:path'); +const args = process.argv.slice(2); +const headerArg = args[args.indexOf('-H') + 1]; +const headerFile = headerArg && headerArg.startsWith('@') ? headerArg.slice(1) : null; +const responseFile = args[args.indexOf('-o') + 1]; +const record = { args, headerFile, responseFile }; +if (headerFile) { + record.header = fs.readFileSync(headerFile, 'utf8'); + record.headerMode = fs.statSync(headerFile).mode & 0o777; + record.tempDir = path.dirname(headerFile); +} +fs.writeFileSync(responseFile, process.env.CURL_RESPONSE_BODY); +fs.writeFileSync(process.env.CURL_CALL_LOG, JSON.stringify(record)); +if (Number(process.env.CURL_STATUS)) process.exit(Number(process.env.CURL_STATUS)); +process.stdout.write(process.env.CURL_HTTP_STATUS); +`); fs.chmodSync(path.join(bin, 'curl'), 0o755); - return spawnSync('bash', [SCRIPT, deb], { + result = spawnSync('bash', [SCRIPT, deb], { encoding: 'utf8', env: { HOME: home, PATH: `${bin}:${process.env.PATH}`, + RUNNER_TEMP: runnerTemp, STOKE_CONFIG_FILE: path.join(dir, 'missing-config.json'), STOKE_TOKEN: token, + FORGE_URL: 'https://forge.example.test', + CURL_CALL_LOG: log, + CURL_HTTP_STATUS: String(httpStatus), + CURL_RESPONSE_BODY: responseBody, + CURL_STATUS: String(curlStatus), }, }); + call = fs.existsSync(log) ? JSON.parse(fs.readFileSync(log, 'utf8')) : null; + const remainingTempEntries = fs.readdirSync(runnerTemp); + const legacyAfter = fs.readdirSync(os.tmpdir()).filter( + (name) => name.startsWith('stoke-publish-response.') && !legacyBefore.has(name), + ); + + return { + result, + call, + runnerTemp, + remainingTempEntries, + legacyAfter, + headerExistsAfter: call?.headerFile ? fs.existsSync(call.headerFile) : false, + responseExistsAfter: call?.responseFile ? fs.existsSync(call.responseFile) : false, + }; } finally { + if (call?.responseFile && !call.responseFile.startsWith(`${dir}${path.sep}`)) { + fs.rmSync(call.responseFile, { force: true }); + } fs.rmSync(dir, { recursive: true, force: true }); } } +function assertCleaned(scenario) { + assert.deepEqual(scenario.remainingTempEntries, []); + assert.deepEqual(scenario.legacyAfter, []); + assert.equal(scenario.headerExistsAfter, false); + assert.equal(scenario.responseExistsAfter, false); +} + test('empty token identifies the CI secret before offering the local remedy', () => { - const result = runScenario(); + const scenario = runScenario(); - assert.equal(result.status, 1); - assert.equal(result.stdout, ''); - assert.match(result.stderr, /^error: no token\./); - assert.match(result.stderr, /STOKE_TOKEN/); - assert.match(result.stderr, /RELEASE_TOKEN/); - assert.match(result.stderr, /empty value.*secret/is); - assert.ok(result.stderr.indexOf('RELEASE_TOKEN') < result.stderr.indexOf('stoke auth login')); + assert.equal(scenario.result.status, 1); + assert.equal(scenario.result.stdout, ''); + assert.match(scenario.result.stderr, /^error: no token\./); + assert.match(scenario.result.stderr, /STOKE_TOKEN/); + assert.match(scenario.result.stderr, /RELEASE_TOKEN/); + assert.match(scenario.result.stderr, /empty value.*secret/is); + assert.ok(scenario.result.stderr.indexOf('RELEASE_TOKEN') < scenario.result.stderr.indexOf('stoke auth login')); + assert.equal(scenario.call, null); + assertCleaned(scenario); }); -test('non-empty environment token passes the guard without exposing the token', () => { - const result = runScenario({ token: TOKEN }); +test('curl reads a private authorization header file without receiving the token in argv', () => { + const scenario = runScenario({ token: TOKEN }); - assert.equal(result.status, 0, result.stderr); - assert.match(result.stdout, /Published\./); - assert.doesNotMatch(result.stdout, new RegExp(TOKEN)); - assert.doesNotMatch(result.stderr, new RegExp(TOKEN)); + assert.equal(scenario.result.status, 0, scenario.result.stderr); + assert.ok(scenario.call.args.includes('-H')); + assert.equal(scenario.call.args.every((arg) => !arg.includes(TOKEN)), true); + assert.equal(path.dirname(scenario.call.tempDir), scenario.runnerTemp); + assert.ok(scenario.call.headerFile.startsWith(`${scenario.call.tempDir}${path.sep}`)); + assert.equal(scenario.call.header, `Authorization: token ${TOKEN}\n`); + assert.equal(scenario.call.headerMode, 0o600); + assert.equal(path.dirname(scenario.call.responseFile), scenario.call.tempDir); + assert.doesNotMatch(scenario.result.stdout, new RegExp(TOKEN)); + assert.doesNotMatch(scenario.result.stderr, new RegExp(TOKEN)); + assertCleaned(scenario); +}); + +test('201 response preserves the success transcript and removes temporary files', () => { + const scenario = runScenario({ token: TOKEN, httpStatus: 201 }); + + assert.equal(scenario.result.status, 0, scenario.result.stderr); + assert.equal(scenario.result.stdout, `${UPLOAD_LINE}Published.\n`); + assert.equal(scenario.result.stderr, ''); + assertCleaned(scenario); +}); + +test('409 response preserves the already-published transcript and removes temporary files', () => { + const scenario = runScenario({ token: TOKEN, httpStatus: 409 }); + + assert.equal(scenario.result.status, 0, scenario.result.stderr); + assert.equal(scenario.result.stdout, `${UPLOAD_LINE}Already published (409): this exact version already exists in the registry.\n`); + assert.equal(scenario.result.stderr, ''); + assertCleaned(scenario); +}); + +test('HTTP failure preserves the response body on stderr and removes temporary files', () => { + const scenario = runScenario({ token: TOKEN, httpStatus: 500, responseBody: 'registry rejected\n' }); + + assert.equal(scenario.result.status, 1); + assert.equal(scenario.result.stdout, UPLOAD_LINE); + assert.equal(scenario.result.stderr, 'error: upload failed with HTTP 500\nregistry rejected\n'); + assertCleaned(scenario); +}); + +test('curl failure propagates its status and still removes temporary files', () => { + const scenario = runScenario({ token: TOKEN, curlStatus: 7, responseBody: 'transport failed\n' }); + + assert.equal(scenario.result.status, 7); + assert.equal(scenario.result.stdout, UPLOAD_LINE); + assert.equal(scenario.result.stderr, ''); + assertCleaned(scenario); });