refactor: newsletter signup posts no credential (SEC-14-R1 option a)
Rework PR #15 per security review SEC-14-R1: the client no longer carries an API token. js/newsletter-config.js ships only the non-secret endpoint, enforced https-only at config load time via validateEndpoint() (mirroring the protocol allowlist in js/reading-list.js); js/newsletter.js POSTs email-only with no Authorization header. Failure paths keep the single user-safe message that never leaks token, endpoint, status, or raw body; success still shows the confirmation. CI gains a gitleaks step that fails on any secret hit; README documents the server-side token, the residual signup-abuse risk, and the authoritative server-side validation follow-up.
This commit is contained in:
implementer
+178
-109
@@ -1,6 +1,6 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { readdirSync, readFileSync, statSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
import {
|
||||
@@ -12,8 +12,9 @@ import {
|
||||
submitNewsletterSignup,
|
||||
} from "../js/newsletter.js";
|
||||
import {
|
||||
NEWSLETTER_API_TOKEN,
|
||||
NEWSLETTER_ENDPOINT,
|
||||
isHttpsUrl,
|
||||
validateEndpoint,
|
||||
} from "../js/newsletter-config.js";
|
||||
|
||||
const root = join(dirname(fileURLToPath(import.meta.url)), "..");
|
||||
@@ -34,7 +35,7 @@ function fakeForm(email) {
|
||||
};
|
||||
}
|
||||
|
||||
/** Record calls and respond — pass an object or a (url, init) => response fn. */
|
||||
/** Record fetch calls; respond with an object or via a (url, init) => response fn. */
|
||||
function fakeFetch(respond) {
|
||||
const calls = [];
|
||||
const impl = async (url, init) => {
|
||||
@@ -53,20 +54,20 @@ test("newsletter page renders an email field and a submit button", () => {
|
||||
assert.match(newsletterHtml, /<form[^>]*id="newsletter-form"/);
|
||||
assert.match(
|
||||
newsletterHtml,
|
||||
/<input[^>]*type="email"[^>]*id="newsletter-email"[^>]*name="email"/
|
||||
/<input[^>]*type="email"[^>]*id="newsletter-email"[^>]*name="email"/,
|
||||
);
|
||||
assert.match(newsletterHtml, /<button[^>]*type="submit"[^>]*>Subscribe<\/button>/);
|
||||
});
|
||||
|
||||
test("newsletter email field is required and the page loads the wiring module", () => {
|
||||
test("the email field is required and the page loads the wiring module", () => {
|
||||
assert.match(newsletterHtml, /<input[^>]*id="newsletter-email"[^>]*required/);
|
||||
assert.match(
|
||||
newsletterHtml,
|
||||
/<script[^>]*type="module"[^>]*src="js\/newsletter\.js"/
|
||||
/<script[^>]*type="module"[^>]*src="js\/newsletter\.js"/,
|
||||
);
|
||||
});
|
||||
|
||||
test("newsletter page has a live status region for the result message", () => {
|
||||
test("the newsletter page has a live status region for results", () => {
|
||||
assert.match(newsletterHtml, /id="newsletter-status"/);
|
||||
assert.match(newsletterHtml, /role="status"/);
|
||||
assert.match(newsletterHtml, /aria-live="polite"/);
|
||||
@@ -78,50 +79,110 @@ test("every site page links to the newsletter page, and it marks itself current"
|
||||
}
|
||||
assert.match(
|
||||
newsletterHtml,
|
||||
/<a href="newsletter\.html"[^>]*aria-current="page"[^>]*>Newsletter<\/a>/
|
||||
/<a href="newsletter\.html"[^>]*aria-current="page"[^>]*>Newsletter<\/a>/,
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Config & secrets: token comes from config, never hardcoded in source
|
||||
// Endpoint config: https-only, enforced at config/test time (mirrors the
|
||||
// protocol allowlist pattern in js/reading-list.js)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("endpoint and token are imported from the config module, never defined in the page logic", () => {
|
||||
assert.match(
|
||||
newsletterJs,
|
||||
/import\s*\{[\s\S]*NEWSLETTER_ENDPOINT[\s\S]*\}\s*from\s*"\.\/newsletter-config\.js"/
|
||||
);
|
||||
assert.match(
|
||||
newsletterJs,
|
||||
/import\s*\{[\s\S]*NEWSLETTER_API_TOKEN[\s\S]*\}\s*from\s*"\.\/newsletter-config\.js"/
|
||||
);
|
||||
// The page logic must not assign a token literal itself.
|
||||
assert.doesNotMatch(newsletterJs, /NEWSLETTER_API_TOKEN\s*=\s*["'][^"']+["']/);
|
||||
test("isHttpsUrl accepts https and rejects every other scheme", () => {
|
||||
assert.equal(isHttpsUrl("https://api.example.com/newsletter"), true);
|
||||
assert.equal(isHttpsUrl("https://example.com"), true);
|
||||
assert.equal(isHttpsUrl("http://api.example.com/newsletter"), false);
|
||||
assert.equal(isHttpsUrl("javascript:alert(1)"), false);
|
||||
assert.equal(isHttpsUrl("ftp://example.com/newsletter"), false);
|
||||
assert.equal(isHttpsUrl("not a url"), false);
|
||||
assert.equal(isHttpsUrl(""), false);
|
||||
assert.equal(isHttpsUrl(undefined), false);
|
||||
});
|
||||
|
||||
test("the committed config carries an empty placeholder token, so no secret is committed", () => {
|
||||
assert.equal(NEWSLETTER_API_TOKEN, "");
|
||||
assert.equal(typeof NEWSLETTER_ENDPOINT, "string");
|
||||
test("validateEndpoint throws on a non-https endpoint and accepts an https one", () => {
|
||||
assert.equal(validateEndpoint("https://api.example.com/newsletter"), true);
|
||||
assert.throws(() => validateEndpoint("http://api.example.com/newsletter"), {
|
||||
message: /https/,
|
||||
});
|
||||
assert.throws(() => validateEndpoint("ftp://example.com/newsletter"), {
|
||||
message: /https/,
|
||||
});
|
||||
});
|
||||
|
||||
test("the configured endpoint is https at config load time (config-time assertion)", () => {
|
||||
// Importing the module already runs validateEndpoint(NEWSLETTER_ENDPOINT);
|
||||
// this asserts the shipped value satisfies the same rule.
|
||||
assert.equal(isHttpsUrl(NEWSLETTER_ENDPOINT), true);
|
||||
assert.ok(NEWSLETTER_ENDPOINT.startsWith("https://"));
|
||||
assert.doesNotThrow(() => validateEndpoint(NEWSLETTER_ENDPOINT));
|
||||
});
|
||||
|
||||
test("no shipped page or script embeds a secret-shaped token literal", () => {
|
||||
const secretShape = /["'][A-Za-z0-9+/_-]{32,}["']/;
|
||||
for (const source of [newsletterJs, configJs, newsletterHtml]) {
|
||||
assert.doesNotMatch(source, secretShape);
|
||||
// ---------------------------------------------------------------------------
|
||||
// Secrets: no API token or secret in any committed source, fixture, or
|
||||
// client-served asset (whole-tree scan)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("the page logic never sends or references a credential", () => {
|
||||
assert.ok(!newsletterJs.includes("Authorization"));
|
||||
assert.ok(!newsletterJs.includes("Bearer"));
|
||||
assert.ok(!newsletterJs.includes("NEWSLETTER_API_TOKEN"));
|
||||
assert.ok(!newsletterJs.includes("api_key"));
|
||||
});
|
||||
|
||||
test("the config module ships the endpoint only — no token export or literal", () => {
|
||||
assert.ok(!configJs.includes("NEWSLETTER_API_TOKEN"));
|
||||
assert.ok(!configJs.includes("Authorization"));
|
||||
assert.ok(!configJs.includes("Bearer"));
|
||||
assert.ok(!configJs.match(/token\s*[:=]\s*["'][^"']{4,}["']/i));
|
||||
});
|
||||
|
||||
test("no secret-shaped literal ships anywhere in the tree (whole-tree scan)", () => {
|
||||
const patterns = [
|
||||
/["'][A-Za-z0-9+/_]{32,}["']/, // long opaque strings (tokens, keys)
|
||||
/-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----/,
|
||||
/\b(ghp|gho|github_pat|glpat)_[A-Za-z0-9_]{20,}\b/,
|
||||
/\bsk-[A-Za-z0-9]{20,}\b/,
|
||||
/\bxox[baprs]-[A-Za-z0-9-]{10,}\b/,
|
||||
/\bAKIA[0-9A-Z]{16}\b/,
|
||||
/\bAIza[0-9A-Za-z_-]{35}\b/,
|
||||
/["']Bearer\s+[^"'\s]{8,}["']/,
|
||||
];
|
||||
|
||||
const files = [];
|
||||
const walk = (dir) => {
|
||||
for (const entry of readdirSync(dir)) {
|
||||
if (entry === ".git" || entry === "node_modules") continue;
|
||||
const full = join(dir, entry);
|
||||
if (statSync(full).isDirectory()) walk(full);
|
||||
else files.push(full);
|
||||
}
|
||||
};
|
||||
walk(root);
|
||||
|
||||
assert.ok(files.length > 10, "whole-tree scan should cover the repo");
|
||||
for (const file of files) {
|
||||
const source = readFileSync(file, "utf8");
|
||||
for (const pattern of patterns) {
|
||||
assert.doesNotMatch(source, pattern, `${file} contains a secret-shaped literal`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test("CI runs a gitleaks step that fails on any secret hit", () => {
|
||||
const ci = readFileSync(join(root, ".gitea/workflows/ci.yml"), "utf8");
|
||||
assert.match(ci, /gitleaks/i);
|
||||
assert.match(ci, /detect/i);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Unit tests: submitting POSTs to the configured serverless endpoint
|
||||
// Submission: POSTs to the configured https-only endpoint with no credential
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("submitNewsletterSignup POSTs the email to the endpoint with the token in the Authorization header", async () => {
|
||||
test("submitNewsletterSignup POSTs the email with no Authorization header or credential", async () => {
|
||||
const fetchImpl = fakeFetch({ ok: true });
|
||||
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token: "test-token-123",
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
@@ -131,86 +192,39 @@ test("submitNewsletterSignup POSTs the email to the endpoint with the token in t
|
||||
assert.equal(url, "https://api.example.com/newsletter");
|
||||
assert.equal(init.method, "POST");
|
||||
assert.equal(init.headers["Content-Type"], "application/json");
|
||||
assert.equal(init.headers["Authorization"], "Bearer test-token-123");
|
||||
assert.equal(init.headers.Authorization, undefined);
|
||||
assert.ok(
|
||||
!Object.keys(init.headers).some((h) => h.toLowerCase() === "authorization"),
|
||||
"request must not carry an Authorization header",
|
||||
);
|
||||
assert.deepEqual(JSON.parse(init.body), { email: "ada@example.com" });
|
||||
});
|
||||
|
||||
test("submitNewsletterSignup resolves success with a confirmation message on a 2xx response", async () => {
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token: "test-token-123",
|
||||
fetchImpl: fakeFetch({ ok: true, status: 200 }),
|
||||
});
|
||||
assert.deepEqual(result, { ok: true, message: NEWSLETTER_SUCCESS_MESSAGE });
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Unit tests: missing or invalid token → user-safe error, no secret leak
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("a missing token fails with a user-safe error and never calls the network", async () => {
|
||||
test("submitNewsletterSignup defaults to the configured endpoint", async () => {
|
||||
const fetchImpl = fakeFetch({ ok: true });
|
||||
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token: "",
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
assert.equal(fetchImpl.calls.length, 0);
|
||||
// The message reveals nothing about the failure cause or configuration.
|
||||
assert.ok(!result.message.includes("token"));
|
||||
assert.ok(!result.message.includes("Bearer"));
|
||||
assert.ok(!result.message.includes("secret"));
|
||||
assert.ok(!result.message.includes("api.example.com"));
|
||||
});
|
||||
|
||||
test("an invalid token (401 from the endpoint) shows a user-safe error without leaking the secret", async () => {
|
||||
const token = "sekrit-token-abc123";
|
||||
const fetchImpl = fakeFetch({ ok: false, status: 401 });
|
||||
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token,
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
assert.ok(!result.message.includes(token));
|
||||
assert.ok(!result.message.includes("401"));
|
||||
assert.ok(!result.message.includes("api.example.com"));
|
||||
});
|
||||
|
||||
test("a network failure shows the same user-safe error", async () => {
|
||||
const fetchImpl = fakeFetch(() => {
|
||||
throw new Error("network down");
|
||||
});
|
||||
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token: "test-token-123",
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
await submitNewsletterSignup("ada@example.com", { fetchImpl });
|
||||
assert.equal(fetchImpl.calls[0].url, NEWSLETTER_ENDPOINT);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Unit tests: form handling wires result → status region
|
||||
// Confirmation: a successful signup resolves and surfaces a confirmation
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("submitNewsletterSignup resolves success with a confirmation message on a 2xx response", async () => {
|
||||
for (const status of [200, 201, 204]) {
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
fetchImpl: fakeFetch({ ok: true, status }),
|
||||
});
|
||||
assert.deepEqual(result, { ok: true, message: NEWSLETTER_SUCCESS_MESSAGE });
|
||||
}
|
||||
});
|
||||
|
||||
test("handleNewsletterSubmit shows the confirmation message after a successful signup", async () => {
|
||||
const fetchImpl = fakeFetch({ ok: true });
|
||||
const statuses = [];
|
||||
|
||||
const result = await handleNewsletterSubmit(fakeForm("ada@example.com"), {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token: "test-token-123",
|
||||
fetchImpl,
|
||||
fetchImpl: fakeFetch({ ok: true }),
|
||||
setStatus: (message, kind) => statuses.push({ message, kind }),
|
||||
});
|
||||
|
||||
@@ -219,34 +233,89 @@ test("handleNewsletterSubmit shows the confirmation message after a successful s
|
||||
assert.deepEqual(statuses, [
|
||||
{ message: NEWSLETTER_SUCCESS_MESSAGE, kind: "success" },
|
||||
]);
|
||||
assert.equal(fetchImpl.calls.length, 1);
|
||||
});
|
||||
|
||||
test("handleNewsletterSubmit rejects an invalid email without any network call", async () => {
|
||||
const fetchImpl = fakeFetch({ ok: true });
|
||||
const statuses = [];
|
||||
// ---------------------------------------------------------------------------
|
||||
// Failure: a failed or unavailable submission surfaces a user-safe error that
|
||||
// never leaks the server-held token, the endpoint, the status, or any raw body
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const result = await handleNewsletterSubmit(fakeForm("not-an-email"), {
|
||||
token: "test-token-123",
|
||||
test("a failed submission shows a user-safe error that never leaks token, endpoint, status, or raw body", async () => {
|
||||
const endpoint = "https://api.example.com/newsletter";
|
||||
// A raw body full of fragments the visitor must never see: a token-shaped
|
||||
// value, the endpoint host, and status codes.
|
||||
const rawBody = '{"error":"unauthorized","token":"example-secret-value","detail":"api.example.com 500"}';
|
||||
const fragments = [
|
||||
"example-secret-value",
|
||||
"api.example.com",
|
||||
"500",
|
||||
"Bearer",
|
||||
"token",
|
||||
"secret",
|
||||
];
|
||||
|
||||
for (const status of [400, 401, 403, 404, 429, 500, 503]) {
|
||||
const fetchImpl = fakeFetch({ ok: false, status, text: async () => rawBody });
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint,
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
for (const fragment of fragments) {
|
||||
assert.ok(
|
||||
!result.message.toLowerCase().includes(fragment.toLowerCase()),
|
||||
`message must not contain "${fragment}"`,
|
||||
);
|
||||
}
|
||||
assert.equal(fetchImpl.calls.length, 1);
|
||||
}
|
||||
});
|
||||
|
||||
test("a network failure shows the same user-safe error", async () => {
|
||||
const fetchImpl = fakeFetch(() => {
|
||||
throw new Error("network down");
|
||||
});
|
||||
const result = await submitNewsletterSignup("ada@example.com", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
fetchImpl,
|
||||
});
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
});
|
||||
|
||||
test("an invalid email fails fast with a user-safe error and no network call", async () => {
|
||||
const fetchImpl = fakeFetch({ ok: true });
|
||||
const result = await submitNewsletterSignup("not-an-email", {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
fetchImpl,
|
||||
});
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
assert.equal(fetchImpl.calls.length, 0);
|
||||
});
|
||||
|
||||
test("handleNewsletterSubmit surfaces a user-safe error for a failed submission", async () => {
|
||||
const statuses = [];
|
||||
const result = await handleNewsletterSubmit(fakeForm("ada@example.com"), {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
fetchImpl: fakeFetch({ ok: false, status: 500 }),
|
||||
setStatus: (message, kind) => statuses.push({ message, kind }),
|
||||
});
|
||||
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.message, NEWSLETTER_ERROR_MESSAGE);
|
||||
assert.equal(fetchImpl.calls.length, 0);
|
||||
assert.deepEqual(statuses, [
|
||||
{ message: NEWSLETTER_ERROR_MESSAGE, kind: "error" },
|
||||
]);
|
||||
});
|
||||
|
||||
test("handleNewsletterSubmit surfaces a user-safe error when the config token is missing", async () => {
|
||||
test("handleNewsletterSubmit rejects an invalid email with no network call", async () => {
|
||||
const fetchImpl = fakeFetch({ ok: true });
|
||||
const statuses = [];
|
||||
|
||||
const result = await handleNewsletterSubmit(fakeForm("ada@example.com"), {
|
||||
const result = await handleNewsletterSubmit(fakeForm("not-an-email"), {
|
||||
endpoint: "https://api.example.com/newsletter",
|
||||
token: "",
|
||||
fetchImpl,
|
||||
setStatus: (message, kind) => statuses.push({ message, kind }),
|
||||
});
|
||||
@@ -260,7 +329,7 @@ test("handleNewsletterSubmit surfaces a user-safe error when the config token is
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Unit tests: helpers
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("readFormEmail returns the trimmed email value", () => {
|
||||
|
||||
Reference in New Issue
Block a user