[object Object]

← back to Secrets Manager

Harden secret freshness verification failures

f585023e4d19be278f28b7b946646310b373db2d · 2026-08-28 22:22:48 -0700 · Steve Abrams

Files touched

Diff

commit f585023e4d19be278f28b7b946646310b373db2d
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Fri Aug 28 22:22:48 2026 -0700

    Harden secret freshness verification failures
---
 cli.js                      | 49 ++++++++++++++++++++++++++++++++++++---------
 test/verify-all.test.js     | 40 +++++++++++++++++++++++++++++++++++-
 verification/e2e-proof.json |  6 ++++--
 3 files changed, 83 insertions(+), 12 deletions(-)

diff --git a/cli.js b/cli.js
index 59e5a24..6445cde 100755
--- a/cli.js
+++ b/cli.js
@@ -275,13 +275,13 @@ function configuredSecretKeys(routes = ROUTES) {
   return [...keys].sort();
 }
 
-async function buildVerifyAllReport({ master, routes = ROUTES, verifier = verifyToken, now = new Date() }) {
+async function buildVerifyAllReport({ master, routes = ROUTES, verifier = verifyToken, now = new Date(), timeoutMs = 10000 }) {
   const checks = [];
   const lookup = (key) => (routes.services && routes.services[key]) || routes[key] || null;
   for (const key of configuredSecretKeys(routes)) {
     const cfg = lookup(key);
     const value = master[key];
-    if (!value) {
+    if (typeof value !== 'string' || !value.trim()) {
       checks.push({ key, outcome: cfg?.verify ? 'FAIL' : 'WARN', reason: 'missing-from-master' });
       continue;
     }
@@ -289,10 +289,10 @@ async function buildVerifyAllReport({ master, routes = ROUTES, verifier = verify
       checks.push({ key, outcome: 'WARN', reason: 'no-verify-endpoint' });
       continue;
     }
-    const result = await verifier(key, value);
+    const result = await boundedVerify(verifier, key, value, timeoutMs);
     checks.push(result.ok
       ? { key, outcome: 'PASS', http_status: result.status || null }
-      : { key, outcome: 'FAIL', http_status: result.status || null, reason: result.error ? 'network-error' : 'provider-rejected' });
+      : { key, outcome: 'FAIL', http_status: result.status || null, reason: result.reason || 'provider-rejected' });
   }
   const counts = checks.reduce((acc, check) => { acc[check.outcome]++; return acc; }, { PASS: 0, WARN: 0, FAIL: 0 });
   return {
@@ -304,18 +304,43 @@ async function buildVerifyAllReport({ master, routes = ROUTES, verifier = verify
   };
 }
 
+async function boundedVerify(verifier, key, value, timeoutMs = 10000) {
+  let timer;
+  const timeout = new Promise((resolve) => {
+    timer = setTimeout(() => resolve({ ok: false, reason: 'timeout' }), timeoutMs);
+  });
+  try {
+    const result = await Promise.race([
+      Promise.resolve().then(() => verifier(key, value)).catch(() => ({ ok: false, reason: 'verifier-error' })),
+      timeout,
+    ]);
+    if (!result || typeof result !== 'object' || typeof result.ok !== 'boolean') {
+      return { ok: false, reason: 'malformed-result' };
+    }
+    if (!result.ok && !result.reason) {
+      return { ok: false, status: result.status, reason: result.error ? 'network-error' : 'provider-rejected' };
+    }
+    return result;
+  } finally {
+    clearTimeout(timer);
+  }
+}
+
 async function cmdVerifyAll(options = {}) {
   const report = await buildVerifyAllReport({
     master: options.master || loadEnvFile(MASTER_ENV),
     routes: options.routes || ROUTES,
     verifier: options.verifier || verifyToken,
     now: options.now || new Date(),
+    timeoutMs: options.timeoutMs,
   });
   const outputPath = options.outputPath || path.join(ROOT, 'data', 'latest.json');
   fs.mkdirSync(path.dirname(outputPath), { recursive: true });
   const tempPath = `${outputPath}.tmp-${process.pid}`;
-  fs.writeFileSync(tempPath, JSON.stringify(report, null, 2) + '\n');
+  fs.writeFileSync(tempPath, JSON.stringify(report, null, 2) + '\n', { mode: 0o600 });
+  fs.chmodSync(tempPath, 0o600);
   fs.renameSync(tempPath, outputPath);
+  fs.chmodSync(outputPath, 0o600);
   console.log(`verify-all ${report.status} — PASS=${report.summary.PASS} WARN=${report.summary.WARN} FAIL=${report.summary.FAIL}`);
   console.log(`report: ${outputPath}`);
   return report;
@@ -616,14 +641,17 @@ function cmdRegen(args) {
 }
 
 // ─── dispatch ───────────────────────────────────────────────────────────
-async function main(argv = process.argv.slice(2)) {
+async function main(argv = process.argv.slice(2), options = {}) {
   const [cmd, ...rest] = argv;
   switch (cmd) {
     case 'add':           await cmdAdd(rest[0], rest[1]); break;
     case 'import-paste':  await cmdImportPaste(); break;
     case 'list':          cmdList(); break;
     case 'check':         await cmdCheck(rest[0]); break;
-    case 'verify-all':    await cmdVerifyAll(); break;
+    case 'verify-all': {
+      const report = await cmdVerifyAll(options.verifyAllOptions);
+      return report.status === 'FAIL' ? 1 : 0;
+    }
     case 'sync':          cmdSync(); break;
     case 'audit':         cmdAudit(); break;
     case 'regen':         cmdRegen(rest); break;
@@ -631,8 +659,11 @@ async function main(argv = process.argv.slice(2)) {
       console.error('usage: cli.js <add|import-paste|list|check|verify-all|sync|audit|regen> [args]');
       process.exit(2);
   }
+  return 0;
 }
 
-if (require.main === module) main().catch(e => { console.error('ERR:', e.message); process.exit(1); });
+if (require.main === module) main()
+  .then((code) => { process.exitCode = code; })
+  .catch(e => { console.error('ERR:', e.message); process.exitCode = 1; });
 
-module.exports = { buildVerifyAllReport, cmdVerifyAll, configuredSecretKeys };
+module.exports = { boundedVerify, buildVerifyAllReport, cmdVerifyAll, configuredSecretKeys, main };
diff --git a/test/verify-all.test.js b/test/verify-all.test.js
index 41ba9f1..3c121ed 100644
--- a/test/verify-all.test.js
+++ b/test/verify-all.test.js
@@ -5,7 +5,7 @@ const assert = require('node:assert/strict');
 const fs = require('node:fs');
 const os = require('node:os');
 const path = require('node:path');
-const { buildVerifyAllReport, cmdVerifyAll } = require('../cli.js');
+const { buildVerifyAllReport, cmdVerifyAll, main } = require('../cli.js');
 
 const routes = {
   _comment: { text: 'metadata is not a secret' },
@@ -51,6 +51,8 @@ test('writes the latest report atomically for local consumers without network ca
   const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'secret-freshness-'));
   t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
   const outputPath = path.join(dir, 'data', 'latest.json');
+  fs.mkdirSync(path.dirname(outputPath), { recursive: true });
+  fs.writeFileSync(outputPath, '{}\n', { mode: 0o644 });
   const report = await cmdVerifyAll({
     master: { GOOD_KEY: 'x' }, routes: { services: { GOOD_KEY: { verify: {} } } },
     verifier: async () => ({ ok: true, status: 200 }), outputPath,
@@ -58,4 +60,40 @@ test('writes the latest report atomically for local consumers without network ca
   });
   assert.equal(report.status, 'PASS');
   assert.deepEqual(JSON.parse(fs.readFileSync(outputPath, 'utf8')), report);
+  assert.equal(fs.statSync(outputPath).mode & 0o777, 0o600);
+});
+
+test('timeouts, throws, and malformed results fail safely while later checks continue', async () => {
+  const calls = [];
+  const allRoutes = { services: Object.fromEntries(['A_TIMEOUT', 'B_THROW', 'C_UNDEFINED', 'D_MALFORMED', 'E_PASS']
+    .map((key) => [key, { verify: {} }])) };
+  const allMaster = Object.fromEntries(Object.keys(allRoutes.services).map((key) => [key, `value-${key}`]));
+  const report = await buildVerifyAllReport({
+    master: allMaster, routes: allRoutes, timeoutMs: 5,
+    verifier: async (key) => {
+      calls.push(key);
+      if (key === 'A_TIMEOUT') return new Promise(() => {});
+      if (key === 'B_THROW') throw new Error('secret value-B_THROW');
+      if (key === 'C_UNDEFINED') return undefined;
+      if (key === 'D_MALFORMED') return { ok: 'yes', body: 'secret malformed body' };
+      return { ok: true, status: 200, body: 'secret body' };
+    },
+  });
+  assert.deepEqual(calls, ['A_TIMEOUT', 'B_THROW', 'C_UNDEFINED', 'D_MALFORMED', 'E_PASS']);
+  assert.deepEqual(report.checks.map((check) => check.reason || 'pass'),
+    ['timeout', 'verifier-error', 'malformed-result', 'malformed-result', 'pass']);
+  assert.equal(report.status, 'FAIL');
+  assert.doesNotMatch(JSON.stringify(report), /value-|secret body|B_THROW.*secret/);
+});
+
+test('blank secrets count as missing and CLI exit code fails only aggregate FAIL', async (t) => {
+  const base = fs.mkdtempSync(path.join(os.tmpdir(), 'secret-exit-'));
+  t.after(() => fs.rmSync(base, { recursive: true, force: true }));
+  const run = (name, masterValue, route, verifier) => main(['verify-all'], { verifyAllOptions: {
+    master: { TEST_KEY: masterValue }, routes: { services: { TEST_KEY: route } }, verifier,
+    outputPath: path.join(base, `${name}.json`), timeoutMs: 5,
+  } });
+  assert.equal(await run('fail', '   ', { verify: {} }, async () => ({ ok: true })), 1);
+  assert.equal(await run('warn', 'value', {}, async () => ({ ok: true })), 0);
+  assert.equal(await run('pass', 'value', { verify: {} }, async () => ({ ok: true, status: 200 })), 0);
 });
diff --git a/verification/e2e-proof.json b/verification/e2e-proof.json
index 726c562..a357586 100644
--- a/verification/e2e-proof.json
+++ b/verification/e2e-proof.json
@@ -2,16 +2,18 @@
   "intent": "Produce a machine-readable PASS/WARN/FAIL freshness report for configured secrets without exposing secret values.",
   "risk_tier": "R1 isolated CLI/report code; live provider verification and scheduling were not invoked",
   "environment": "local Node tests with injected zero-network verifier and temporary output path",
-  "timestamp": "2026-08-29T05:21:00Z",
+  "timestamp": "2026-08-29T05:28:00Z",
   "ticket": "TK-10950-add-read-only-secret-freshness-report",
   "build_identity": "git parent b712be4 plus owned Cycle 13 diff",
   "checks": [
     { "verdict": "PASS", "boundary": "classification", "command": "node --test test/verify-all.test.js", "assertions": "PASS/WARN/FAIL aggregation covers verified, rejected, missing, and no-endpoint keys" },
     { "verdict": "PASS", "boundary": "secret redaction", "command": "node --test test/verify-all.test.js", "assertions": "report serialization contains no input secret values or provider bodies" },
     { "verdict": "PASS", "boundary": "report artifact", "command": "node --test test/verify-all.test.js", "assertions": "temporary data/latest.json round-trips to the returned schema" },
+    { "verdict": "PASS", "boundary": "failure isolation", "command": "node --test test/verify-all.test.js", "assertions": "timeout, throw, and malformed verifier results become sanitized FAIL rows and later checks continue" },
+    { "verdict": "PASS", "boundary": "permissions and exit status", "command": "node --test test/verify-all.test.js", "assertions": "report replacement remains 0600; FAIL exits 1 while PASS/WARN exit 0" },
     { "verdict": "PASS", "boundary": "syntax/diff", "command": "node --check cli.js && git diff --check", "assertions": "CLI parses and diff is clean" }
   ],
-  "negative_checks": ["provider rejection", "configured key missing from master", "key lacking a verify endpoint", "provider body containing the secret"],
+  "negative_checks": ["provider rejection", "configured key missing or blank in master", "key lacking a verify endpoint", "provider body containing the secret", "never-settling verifier", "thrown verifier", "undefined verifier result"],
   "side_effects": "temporary test report only; no provider request, secret write, route/registry change, launchd install, restart, deploy, or send",
   "cleanup": "temporary directory removed by test",
   "verdict": "PASS for the local report pipeline"

← 76a6bf3 Add secret freshness report command  ·  back to Secrets Manager  ·  Sanitize verifier report fields 2363f17 →