Merge pull request 'fix: keep Debian publish token out of curl argv' (#67) from build/62-publish-deb-token into main

Reviewed-on: #67
Reviewed-by: glm-bot-andresmgsl <andres+5@heavyduty.builders>
Reviewed-by: claude-bot-andresmgsl <andres+1@heavyduty.builders>
Reviewed-by: kimi-bot-andresmgsl <andres+4@heavyduty.builders>

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).

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}' \
+STATUS="$(curl -sS -o "$RESPONSE_FILE" -w '%{http_code}' \
-  -X PUT -H "Authorization: token $TOKEN" \
+  -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
+    cat "$RESPONSE_FILE" >&2 || true
-    rm -f /tmp/stoke-publish-response.$$
     exit 1
     ;;
 esac
-rm -f /tmp/stoke-publish-response.$$

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(scenario.result.status, 1);
-  assert.equal(result.stdout, '');
+  assert.equal(scenario.result.stdout, '');
-  assert.match(result.stderr, /^error: no token\./);
+  assert.match(scenario.result.stderr, /^error: no token\./);
-  assert.match(result.stderr, /STOKE_TOKEN/);
+  assert.match(scenario.result.stderr, /STOKE_TOKEN/);
-  assert.match(result.stderr, /RELEASE_TOKEN/);
+  assert.match(scenario.result.stderr, /RELEASE_TOKEN/);
-  assert.match(result.stderr, /empty value.*secret/is);
+  assert.match(scenario.result.stderr, /empty value.*secret/is);
-  assert.ok(result.stderr.indexOf('RELEASE_TOKEN') < result.stderr.indexOf('stoke auth login'));
+  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', () => {
+test('curl reads a private authorization header file without receiving the token in argv', () => {
-  const result = runScenario({ token: TOKEN });
+  const scenario = runScenario({ token: TOKEN });
 
-  assert.equal(result.status, 0, result.stderr);
+  assert.equal(scenario.result.status, 0, scenario.result.stderr);
-  assert.match(result.stdout, /Published\./);
+  assert.ok(scenario.call.args.includes('-H'));
-  assert.doesNotMatch(result.stdout, new RegExp(TOKEN));
+  assert.equal(scenario.call.args.every((arg) => !arg.includes(TOKEN)), true);
-  assert.doesNotMatch(result.stderr, new RegExp(TOKEN));
+  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);
 });