SECURITY (contrarian FIX FIRST): magic-link signing secret now 256-bit from secrets master — was derived from public ADMIN_PASS (DW2024!) so every approve token was forgeable from the public /trade/apply id. Fail-closed when unset; email falls back to admin panel; dead-letter failed notifies so applications never rot silently. Verified: forged old-secret token → 403, properly-signed → valid.

3561f8647d43b8f5af43932655ff0456b50eb30f · 2026-07-28 14:01:01 -0700 · Steve Abrams

Files touched

M lib/config.js
M lib/trade.js
M server.js

Diff

commit 3561f8647d43b8f5af43932655ff0456b50eb30f
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Tue Jul 28 14:01:01 2026 -0700

    SECURITY (contrarian FIX FIRST): magic-link signing secret now 256-bit from secrets master — was derived from public ADMIN_PASS (DW2024!) so every approve token was forgeable from the public /trade/apply id. Fail-closed when unset; email falls back to admin panel; dead-letter failed notifies so applications never rot silently. Verified: forged old-secret token → 403, properly-signed → valid.
---
 lib/config.js | 10 +++++++---
 lib/trade.js  |  9 ++++++++-
 server.js     | 23 ++++++++++++++++++++---
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/lib/config.js b/lib/config.js
index 74af2b3..677c783 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -90,9 +90,13 @@ const config = {
   // Public base the Approve/Reject buttons point at. Empty → server.js falls back to
   // the local dev port; at go-live set to the Kamatera host (signup.designer…).
   PUBLIC_URL: (process.env.PUBLIC_URL || '').replace(/\/+$/, ''),
-  // Secret signing the one-click approve/reject magic-links. Derived from ADMIN_PASS
-  // when unset so emailed links stay valid across restarts (no random-per-boot key).
-  APPROVE_LINK_SECRET: firstEnv('APPROVE_LINK_SECRET', SECRETS_ENVS) || ('dw-trade::' + (process.env.ADMIN_PASS || 'DW2024!')),
+  // Secret signing the one-click approve/reject magic-links. MUST come from the secrets
+  // master (256-bit random) — NEVER derived from ADMIN_PASS or any documented value, or
+  // an attacker who knows the fleet-standard password could forge an approve token from
+  // the public /trade/apply id (contrarian critical, 2026-07-28). Empty when unset →
+  // verifyActionToken() fails closed and the magic-link routes 503, so a missing secret
+  // disables one-click approval rather than silently trusting a guessable key.
+  APPROVE_LINK_SECRET: firstEnv('APPROVE_LINK_SECRET', SECRETS_ENVS),
 
   // --- Public webhook hardening (the mint endpoint is public + secret-less) ---
   // 1) URL-token auth: register the webhook at /webhooks/customers/create/<token>.
diff --git a/lib/trade.js b/lib/trade.js
index d29ac12..a47eecd 100644
--- a/lib/trade.js
+++ b/lib/trade.js
@@ -23,12 +23,19 @@ const config = require('./config');
 // secret, so the button in the notification email approves/rejects WITHOUT a login.
 // Token is per-application AND per-action, so an approve link can't be replayed as a
 // reject and vice-versa. Truncated to 32 hex chars (128-bit) — plenty for a link.
+//
+// FAIL-CLOSED (contrarian critical, 2026-07-28): if APPROVE_LINK_SECRET is unset we
+// return null (mint) / false (verify) rather than signing with an empty key — a token
+// signed with '' is trivially forgeable. Callers treat null/false as "magic-links off".
+function secretOk() { return typeof config.APPROVE_LINK_SECRET === 'string' && config.APPROVE_LINK_SECRET.length >= 16; }
 function actionToken(id, action) {
+  if (!secretOk()) return null;
   return crypto.createHmac('sha256', config.APPROVE_LINK_SECRET).update(action + ':' + id).digest('hex').slice(0, 32);
 }
 function verifyActionToken(id, action, token) {
-  if (!token) return false;
+  if (!token || !secretOk()) return false;
   const expected = actionToken(id, action);
+  if (!expected) return false;
   const a = Buffer.from(expected), b = Buffer.from(String(token));
   return a.length === b.length && crypto.timingSafeEqual(a, b);
 }
diff --git a/server.js b/server.js
index ec0cf22..49861b3 100644
--- a/server.js
+++ b/server.js
@@ -125,11 +125,28 @@ function baseUrl() { return config.PUBLIC_URL || `http://127.0.0.1:${config.PORT
 // Send the trade-application review card to the office inbox via George.
 async function notifyTradeApplication(app) {
   const base = baseUrl();
-  const approveUrl = `${base}/admin/trade/${app.id}/approve?token=${trade.actionToken(app.id, 'approve')}`;
-  const rejectUrl = `${base}/admin/trade/${app.id}/reject?token=${trade.actionToken(app.id, 'reject')}`;
   const adminUrl = `${base}/admin/trade`;
+  // Mint one-click magic-links only if a real signing secret is configured; otherwise
+  // fall back to the login-gated admin panel so the email never ships a dead/forgeable link.
+  const at = trade.actionToken(app.id, 'approve');
+  const rt = trade.actionToken(app.id, 'reject');
+  const approveUrl = at ? `${base}/admin/trade/${app.id}/approve?token=${at}` : adminUrl;
+  const rejectUrl = rt ? `${base}/admin/trade/${app.id}/reject?token=${rt}` : adminUrl;
   const { subject, html } = email.tradeApplicationEmail({ app, approveUrl, rejectUrl, adminUrl });
-  return email.sendEmail({ to: config.TRADE_NOTIFY_TO, subject, html, source: 'trade-application' });
+  const r = await email.sendEmail({ to: config.TRADE_NOTIFY_TO, subject, html, source: 'trade-application' });
+  // DEAD-LETTER (contrarian #2, 2026-07-28): sendEmail resolves {ok:false} on a George
+  // outage WITHOUT throwing, so the .catch in the route wouldn't fire — the application
+  // would silently rot unseen. Record every failed notify so a crashed George over a
+  // weekend leaves an auditable trail (and a hook for a pending-apps digest) instead of
+  // a black hole. The application itself is already persisted + visible on /admin/trade.
+  if (r && r.ok === false) {
+    try {
+      require('fs').appendFileSync(require('path').join(__dirname, 'data', 'trade-notify-failures.jsonl'),
+        JSON.stringify({ at: new Date().toISOString(), id: app.id, email: app.email, to: config.TRADE_NOTIFY_TO, error: r.error || r.status }) + '\n');
+    } catch (e) { console.error('[trade] dead-letter write failed:', e.message); }
+    console.error(`[trade] NOTIFY FAILED for ${app.id} (${app.email}) — dead-lettered; review /admin/trade`);
+  }
+  return r;
 }
 
 // Small confirmation page rendered when Approve/Reject is clicked from the email.

← d48cfb0 5x re-run: email-approve-button feature verified clean twice · back to Dw Signup Fulfillment · 5x report: append contrarian gate outcome (critical secret f 44974d4 →