[Story] Newsletter signup (serverless) #14

Closed
opened 2026-08-25 19:10:47 +00:00 by kpcto · 36 comments
Owner

Intent

Add a newsletter signup form. Submissions POST to a serverless function that authenticates with an API token it reads server-side from platform env/secrets, so no backend or authentication server runs on the blog itself and no credential is ever shipped to the client. The client ships only the non-secret endpoint URL.

Acceptance criteria

  • Signup form renders an email field and submit button on the blog
  • Submitting POSTs to the configured https-only serverless endpoint with no credential in the request — the serverless function reads its API token from platform env/secrets, never from client-served config
  • The configured endpoint is enforced https-only at config/test time, mirroring the protocol allowlist in js/reading-list.js
  • No API token or secret is present in any committed source, test fixture, or client-served asset; CI runs a gitleaks step that fails on any hit
  • A failed or unavailable submission surfaces a user-safe error that never leaks the server-held token, the endpoint, the status code, or any raw response body
  • A confirmation message shows after a successful signup
  • The serverless function authoritatively validates submissions before processing — email syntax, length caps, pinned Content-Type, and rejection of unknown fields (required follow-up on the function, not this client-side diff)

Explicitly out of scope

  • No self-hosted email backend or subscriber list management
  • No spam filtering or captcha in this pass; the residual signup-abuse risk is documented in README and mitigated server-side by rate limiting / origin allowlist on the function

Test plan

  • Component test asserts the form renders and POSTs to the endpoint with no Authorization header (no credential sent)
  • Test asserts no secret-shaped literal ships in any source/config/fixture (whole-tree scan)
  • Test asserts the endpoint config enforces https-only (config-time assertion, mirroring js/reading-list.js)
  • Test asserts a failed-request response shows a user-safe error containing neither the token nor any raw response/status/host fragment
  • Test asserts a confirmation message shows after a successful signup
  • CI runs a gitleaks step failing on any secret hit

Rollback note

  • Single page plus config; revert the commit to remove

Owning stream

platform

Risk quadrant

agent-full

## Intent Add a newsletter signup form. Submissions POST to a serverless function that authenticates with an API token it reads server-side from platform env/secrets, so no backend or authentication server runs on the blog itself and no credential is ever shipped to the client. The client ships only the non-secret endpoint URL. ## Acceptance criteria - Signup form renders an email field and submit button on the blog - Submitting POSTs to the configured https-only serverless endpoint with no credential in the request — the serverless function reads its API token from platform env/secrets, never from client-served config - The configured endpoint is enforced https-only at config/test time, mirroring the protocol allowlist in js/reading-list.js - No API token or secret is present in any committed source, test fixture, or client-served asset; CI runs a gitleaks step that fails on any hit - A failed or unavailable submission surfaces a user-safe error that never leaks the server-held token, the endpoint, the status code, or any raw response body - A confirmation message shows after a successful signup - The serverless function authoritatively validates submissions before processing — email syntax, length caps, pinned Content-Type, and rejection of unknown fields (required follow-up on the function, not this client-side diff) ## Explicitly out of scope - No self-hosted email backend or subscriber list management - No spam filtering or captcha in this pass; the residual signup-abuse risk is documented in README and mitigated server-side by rate limiting / origin allowlist on the function ## Test plan - Component test asserts the form renders and POSTs to the endpoint with no Authorization header (no credential sent) - Test asserts no secret-shaped literal ships in any source/config/fixture (whole-tree scan) - Test asserts the endpoint config enforces https-only (config-time assertion, mirroring js/reading-list.js) - Test asserts a failed-request response shows a user-safe error containing neither the token nor any raw response/status/host fragment - Test asserts a confirmation message shows after a successful signup - CI runs a gitleaks step failing on any secret hit ## Rollback note - Single page plus config; revert the commit to remove ### Owning stream platform ### Risk quadrant agent-full
kpcto added the
kind
story
status
ready
labels 2026-08-25 19:11:25 +00:00
bot-dispatcher added
loop
1
status
in-progress
and removed
status
ready
labels 2026-08-25 19:11:25 +00:00
Member
agent: dispatcher
phase: complete
plan_summary: Dispatch #14: [Story] Newsletter signup (serverless) (platform, security-gated)
```yaml agent: dispatcher phase: complete plan_summary: Dispatch #14: [Story] Newsletter signup (serverless) (platform, security-gated) ```
bot-dispatcher added the needs/security-reviewneeds/security-review labels 2026-08-25 19:11:26 +00:00
Member
agent: security-reviewer
phase: start
issue: 14
title: "[Story] Newsletter signup (serverless)"
review_type: security
stage: pre-implementation (no diff/PR found; reviewing story design against main@e24ba89)
playbook: security-audit + security-baseline
checks_planned:
  - secrets exposure (client-shipped API token)
  - input boundaries (email validation, endpoint URL handling)
  - injection / unsafe DOM sinks in new form code
  - error-path information disclosure
  - dependency & CI posture of added code
```yaml agent: security-reviewer phase: start issue: 14 title: "[Story] Newsletter signup (serverless)" review_type: security stage: pre-implementation (no diff/PR found; reviewing story design against main@e24ba89) playbook: security-audit + security-baseline checks_planned: - secrets exposure (client-shipped API token) - input boundaries (email validation, endpoint URL handling) - injection / unsafe DOM sinks in new form code - error-path information disclosure - dependency & CI posture of added code ```
Member
agent: tester
phase: start
issue: 14
objective: "Independently probe the PR for #14 against all 4 acceptance criteria"
target: "Fabrika/PersonalBlog — Newsletter signup (serverless)"
scope_files: []
constraints:
  - "never edit the implementer's tests in the same PR"
plan:
  - "Read issue #14 acceptance criteria + test plan"
  - "Locate the feature branch/PR for #14 and diff vs main"
  - "Verify signup form renders an email field + submit button on the blog"
  - "Verify submit POSTs to the configured serverless endpoint with the API token from config (never hardcoded)"
  - "Verify missing/invalid token surfaces a user-safe error without leaking the secret"
  - "Verify a confirmation message shows after a successful signup"
  - "Reconstruct the tree at the PR head and run npm test independently"
  - "Check CI commit status and post evidence comment with CI links"
```yaml agent: tester phase: start issue: 14 objective: "Independently probe the PR for #14 against all 4 acceptance criteria" target: "Fabrika/PersonalBlog — Newsletter signup (serverless)" scope_files: [] constraints: - "never edit the implementer's tests in the same PR" plan: - "Read issue #14 acceptance criteria + test plan" - "Locate the feature branch/PR for #14 and diff vs main" - "Verify signup form renders an email field + submit button on the blog" - "Verify submit POSTs to the configured serverless endpoint with the API token from config (never hardcoded)" - "Verify missing/invalid token surfaces a user-safe error without leaking the secret" - "Verify a confirmation message shows after a successful signup" - "Reconstruct the tree at the PR head and run npm test independently" - "Check CI commit status and post evidence comment with CI links" ```
Member
agent: implementer
run_id: implementer-14-b22d1e09
issue: 14
phase: started
branch: feature/14
loop: 1
evidence: []
cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"}
notes: "Implementing serverless newsletter signup: newsletter.html form, config-driven endpoint + API token, user-safe error handling with no secret leakage. No self-hosted backend, no spam/captcha (out of scope)."
```yaml agent: implementer run_id: implementer-14-b22d1e09 issue: 14 phase: started branch: feature/14 loop: 1 evidence: [] cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"} notes: "Implementing serverless newsletter signup: newsletter.html form, config-driven endpoint + API token, user-safe error handling with no secret leakage. No self-hosted backend, no spam/captcha (out of scope)." ```
Member
agent: implementer
run_id: implementer-14-b22d1e09
issue: 14
phase: complete
pr: 15
branch: feature/14
loop: 1
evidence:
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15"
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/branch/feature/14/tests/newsletter.test.js"
cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"}
notes: "PR #15 open with criterion→test table. npm test 47/47 green locally on head cb9cf70; Actions runner absent on this instance so CI stays pending (same as prior PRs). Newsletter signup: config-driven endpoint/token, user-safe errors, no secret committed."
```yaml agent: implementer run_id: implementer-14-b22d1e09 issue: 14 phase: complete pr: 15 branch: feature/14 loop: 1 evidence: - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/branch/feature/14/tests/newsletter.test.js" cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"} notes: "PR #15 open with criterion→test table. npm test 47/47 green locally on head cb9cf70; Actions runner absent on this instance so CI stays pending (same as prior PRs). Newsletter signup: config-driven endpoint/token, user-safe errors, no secret committed." ```
Member
agent: reviewer
verdict: approve
```yaml agent: reviewer verdict: approve ```
Member

Reviewer approved; awaiting security verdict before status/review.

> Reviewer approved; awaiting security verdict before status/review.
Member
agent: tester
run_id: tester-14-e24b630c
phase: complete
issue: 14
pr: 15
branch: feature/14
verdict: pass
target: "Fabrika/PersonalBlog PR #15 (feature/14, head cb9cf70) — Newsletter signup (serverless)"
summary: >
  All 4 acceptance criteria verified independently against the PR head. The feature
  adds a newsletter.html signup page (email field + Subscribe button), a config module
  for the endpoint/token, and POST logic that sends the token in an Authorization header
  and maps every failure to one fixed user-safe error. The full suite passes 47/47 in my
  own fresh run; my independent boundary probes (18/19) confirm no secret leak on any
  error path. CI check is "pending / Waiting to run" (no Actions runner on this instance,
  same as accepted PRs #6/#9/#11).

acceptance:
  - criterion: "Signup form renders an email field and submit button on the blog"
    status: pass
    evidence: >
      newsletter.html contains <form id="newsletter-form">, <input type="email"
      id="newsletter-email" name="email" required>, and <button type="submit">Subscribe</button>,
      and loads js/newsletter.js as a module. A "Newsletter" nav link is present on all four
      pages (index.html, reading.html, contact.html, newsletter.html), with aria-current="page"
      on the newsletter page. Verified by direct inspection at the PR head and by the
      implementer's component tests ("newsletter page renders an email field and a submit
      button"; "every site page links to the newsletter page...").
  - criterion: "Submitting POSTs to the configured serverless endpoint with the API token from config"
    status: pass
    evidence: >
      js/newsletter.js imports NEWSLETTER_ENDPOINT and NEWSLETTER_API_TOKEN from
      ./newsletter-config.js (never hardcodes them). submitNewsletterSignup issues
      fetch(endpoint, {method:"POST", headers:{Content-Type:"application/json",
      Authorization:"Bearer <token>"}, body: JSON.stringify({email})}). My independent probe
      recorded the call and confirmed method/URL/headers/body exactly. The committed config
      token is an empty placeholder; git history of js/newsletter-config.js shows it was never
      non-empty, so no secret is committed. Test coverage: "submitNewsletterSignup POSTs ...";
      "endpoint and token are imported from the config module..."; "the committed config carries
      an empty placeholder token..."; "no shipped page or script embeds a secret-shaped token literal".
  - criterion: "A missing or invalid token surfaces a user-safe error without leaking the secret"
    status: pass
    evidence: >
      A missing token returns {ok:false, message: NEWSLETTER_ERROR_MESSAGE} without any network
      call. Non-2xx responses and network failures map to the same generic message
      ("Sorry, the newsletter signup isn't available right now. Please try again later.").
      My probes confirmed, for missing token and for 401/403/404/500/302 and a thrown fetch,
      that the message contains no token, "Bearer", secret, endpoint host, or status code.
      Test coverage: "a missing token fails ... and never calls the network"; "an invalid token
      (401 ...) shows a user-safe error without leaking the secret"; "a network failure shows the
      same user-safe error".
  - criterion: "A confirmation message shows after a successful signup"
    status: pass
    evidence: >
      On a 2xx response submitNewsletterSignup resolves {ok:true, message:"Thanks for subscribing!"},
      and handleNewsletterSubmit routes it through setStatus(...,"success") into #newsletter-status
      (role="status", aria-live="polite", data-kind="success"). My probes confirmed 200, 201 and 204
      all yield the success message and that the handler surfaces it as kind "success".
      Test coverage: "submitNewsletterSignup resolves success with a confirmation message on a 2xx
      response"; "handleNewsletterSubmit shows the confirmation message after a successful signup".

tests:
  suite: "node --test (npm test)"
  result: "47/47 pass, 0 fail, 0 skipped (Node v22.23.2)"
  note: >
    Ran independently from a fresh clone of feature/14 at the PR head (cb9cf70) — not from the
    implementer's claimed run. Matches the implementer's stated 47/47.

probes:
  - >
    Independent throwaway script (not committed; implementer's tests untouched) exercised
    boundary/property cases: missing token, 401/403/404/500/302, network throw, 200/201/204,
    Bearer header + JSON body shape, email-validation edges, and a whole-tree secret-shaped-literal
    scan. 18/19 passed. The single non-pass was my own over-strict expectation: isValidEmail is
    documented as a "simple email shape check" (HTML5 type="email" is the primary gate), so it
    accepts "a..b@example.com". Not a defect against the acceptance criteria.

ci:
  commit_status: "CI / Run tests (pull_request)"
  state: pending
  description: "Waiting to run"
  links:
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/13/jobs/0"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/cb9cf703a870efd31871115a8fdd9f2411067c5d/tests/newsletter.test.js"
  note: >
    This Gitea instance has no Actions runner, so the check stays "Waiting to run" (pending) —
    the same situation as accepted PRs #6, #9, and #11. No failing checks; the suite is verified
    green locally (47/47). The Actions API returns 404 for run/workflow endpoints, consistent with
    no runner being deployed.

findings:
  - severity: info
    note: >
      The API token is necessarily shipped to the client on this static, no-build site, so any
      visitor can read it via view-source. This is inherent to the issue's own design ("no backend
      or authentication server runs on the blog itself") and its test plan ("token read from config,
      never hardcoded"). Forwarded concern for the security review (issue already labeled
      needs/security-review); not a tester acceptance failure.
  - severity: info
    note: >
      handleNewsletterSubmit's invalid-email path reuses the generic "signup isn't available right
      now" message, which is slightly misleading for a malformed email. HTML5 type="email" + required
      is the primary in-browser gate, so this is a defensive fallback only. Non-blocking.

notes:
  - >
    Diff is 9 files / +516 / −0 with no deletions: README.md, contact.html, css/style.css,
    index.html, reading.html (nav link only), plus new js/newsletter-config.js, js/newsletter.js,
    newsletter.html, and tests/newsletter.test.js. No changes to existing tests or unrelated files.
  - >
    Out of scope respected: no self-hosted backend, no subscriber list management, no spam filtering,
    no captcha. Config endpoint is a placeholder (https://example.com/...) and the token is empty;
    the deployer must set both at deploy time (documented in README).
```yaml agent: tester run_id: tester-14-e24b630c phase: complete issue: 14 pr: 15 branch: feature/14 verdict: pass target: "Fabrika/PersonalBlog PR #15 (feature/14, head cb9cf70) — Newsletter signup (serverless)" summary: > All 4 acceptance criteria verified independently against the PR head. The feature adds a newsletter.html signup page (email field + Subscribe button), a config module for the endpoint/token, and POST logic that sends the token in an Authorization header and maps every failure to one fixed user-safe error. The full suite passes 47/47 in my own fresh run; my independent boundary probes (18/19) confirm no secret leak on any error path. CI check is "pending / Waiting to run" (no Actions runner on this instance, same as accepted PRs #6/#9/#11). acceptance: - criterion: "Signup form renders an email field and submit button on the blog" status: pass evidence: > newsletter.html contains <form id="newsletter-form">, <input type="email" id="newsletter-email" name="email" required>, and <button type="submit">Subscribe</button>, and loads js/newsletter.js as a module. A "Newsletter" nav link is present on all four pages (index.html, reading.html, contact.html, newsletter.html), with aria-current="page" on the newsletter page. Verified by direct inspection at the PR head and by the implementer's component tests ("newsletter page renders an email field and a submit button"; "every site page links to the newsletter page..."). - criterion: "Submitting POSTs to the configured serverless endpoint with the API token from config" status: pass evidence: > js/newsletter.js imports NEWSLETTER_ENDPOINT and NEWSLETTER_API_TOKEN from ./newsletter-config.js (never hardcodes them). submitNewsletterSignup issues fetch(endpoint, {method:"POST", headers:{Content-Type:"application/json", Authorization:"Bearer <token>"}, body: JSON.stringify({email})}). My independent probe recorded the call and confirmed method/URL/headers/body exactly. The committed config token is an empty placeholder; git history of js/newsletter-config.js shows it was never non-empty, so no secret is committed. Test coverage: "submitNewsletterSignup POSTs ..."; "endpoint and token are imported from the config module..."; "the committed config carries an empty placeholder token..."; "no shipped page or script embeds a secret-shaped token literal". - criterion: "A missing or invalid token surfaces a user-safe error without leaking the secret" status: pass evidence: > A missing token returns {ok:false, message: NEWSLETTER_ERROR_MESSAGE} without any network call. Non-2xx responses and network failures map to the same generic message ("Sorry, the newsletter signup isn't available right now. Please try again later."). My probes confirmed, for missing token and for 401/403/404/500/302 and a thrown fetch, that the message contains no token, "Bearer", secret, endpoint host, or status code. Test coverage: "a missing token fails ... and never calls the network"; "an invalid token (401 ...) shows a user-safe error without leaking the secret"; "a network failure shows the same user-safe error". - criterion: "A confirmation message shows after a successful signup" status: pass evidence: > On a 2xx response submitNewsletterSignup resolves {ok:true, message:"Thanks for subscribing!"}, and handleNewsletterSubmit routes it through setStatus(...,"success") into #newsletter-status (role="status", aria-live="polite", data-kind="success"). My probes confirmed 200, 201 and 204 all yield the success message and that the handler surfaces it as kind "success". Test coverage: "submitNewsletterSignup resolves success with a confirmation message on a 2xx response"; "handleNewsletterSubmit shows the confirmation message after a successful signup". tests: suite: "node --test (npm test)" result: "47/47 pass, 0 fail, 0 skipped (Node v22.23.2)" note: > Ran independently from a fresh clone of feature/14 at the PR head (cb9cf70) — not from the implementer's claimed run. Matches the implementer's stated 47/47. probes: - > Independent throwaway script (not committed; implementer's tests untouched) exercised boundary/property cases: missing token, 401/403/404/500/302, network throw, 200/201/204, Bearer header + JSON body shape, email-validation edges, and a whole-tree secret-shaped-literal scan. 18/19 passed. The single non-pass was my own over-strict expectation: isValidEmail is documented as a "simple email shape check" (HTML5 type="email" is the primary gate), so it accepts "a..b@example.com". Not a defect against the acceptance criteria. ci: commit_status: "CI / Run tests (pull_request)" state: pending description: "Waiting to run" links: - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/13/jobs/0" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/cb9cf703a870efd31871115a8fdd9f2411067c5d/tests/newsletter.test.js" note: > This Gitea instance has no Actions runner, so the check stays "Waiting to run" (pending) — the same situation as accepted PRs #6, #9, and #11. No failing checks; the suite is verified green locally (47/47). The Actions API returns 404 for run/workflow endpoints, consistent with no runner being deployed. findings: - severity: info note: > The API token is necessarily shipped to the client on this static, no-build site, so any visitor can read it via view-source. This is inherent to the issue's own design ("no backend or authentication server runs on the blog itself") and its test plan ("token read from config, never hardcoded"). Forwarded concern for the security review (issue already labeled needs/security-review); not a tester acceptance failure. - severity: info note: > handleNewsletterSubmit's invalid-email path reuses the generic "signup isn't available right now" message, which is slightly misleading for a malformed email. HTML5 type="email" + required is the primary in-browser gate, so this is a defensive fallback only. Non-blocking. notes: - > Diff is 9 files / +516 / −0 with no deletions: README.md, contact.html, css/style.css, index.html, reading.html (nav link only), plus new js/newsletter-config.js, js/newsletter.js, newsletter.html, and tests/newsletter.test.js. No changes to existing tests or unrelated files. - > Out of scope respected: no self-hosted backend, no subscriber list management, no spam filtering, no captcha. Config endpoint is a placeholder (https://example.com/...) and the token is empty; the deployer must set both at deploy time (documented in README). ```
Member
agent: security-reviewer
phase: complete
issue: 14
title: "[Story] Newsletter signup (serverless)"
review_type: security
stage: pre-implementation design review
scope: >
  No PR or feature branch exists for #14 yet (branches: main, feature/5;
  no open PRs), so this review covers the story's intent and acceptance
  criteria against the current codebase main@e24ba8916da115cff31d21a36e70c07b9c140b17.
  A follow-up security pass on the actual diff is still required when the PR opens.
verdict: changes-requested
blocking_findings: [SEC-14-1]
findings:
  - id: SEC-14-1
    severity: high
    status: blocking
    where: issue #14 acceptance criteria 2 & 3 + test plan item 2 (design)
    what: >
      The site is fully static ("No build step, no backend", README.md). Any
      "API token from config" that the browser sends is delivered to every
      visitor no matter where it is stored (js module, JSON, meta tag) —
      relocating it out of source into a shipped config file changes
      packaging, not secrecy. Acceptance criterion "without leaking the
      secret" and the test "token is read from config, never hardcoded"
      are therefore unsatisfiable as written for confidentiality.
    exploit_path: >
      Visitor opens devtools (or curls the config asset), reads the token,
      then POSTs directly to the serverless endpoint bypassing the form —
      scripted subscriber-list spam and provider quota/billing abuse.
    fix: >
      Pick one BEFORE implementation: (a) preferred — keep the real token
      server-side only; the serverless function reads it from platform
      env/secrets at deploy time (matches security-baseline "secrets live in
      CI/deploy only"), and the page posts nothing secret; or (b) if the
      provider mandates a client-side key, treat it as publishable-by-design:
      rename it accordingly, drop the "without leaking" acceptance criterion,
      document the residual risk, and compensate server-side (rate limiting,
      origin check). Rewrite AC 2/3 accordingly.
  - id: SEC-14-2
    severity: medium
    status: advisory
    where: issue #14 acceptance criteria / test plan (missing requirement)
    what: >
      No validation boundary specified. Client-side email checks are UX only;
      the serverless function must authoritatively validate email syntax,
      enforce length caps, pin content type, and reject unexpected fields.
    exploit_path: >
      Arbitrary oversized or malformed payloads reach the subscription
      provider if only the form validates.
    fix: >
      Add an acceptance criterion + test: server rejects invalid email,
      over-length input, and unknown fields before processing
      (security-baseline: validate at the boundary, reject before parse).
  - id: SEC-14-3
    severity: medium
    status: advisory
    where: issue #14 acceptance criterion 3 (underspecified)
    what: >
      "User-safe error without leaking the secret" needs concrete rules:
      upstream error bodies may echo tokens, endpoints, or internal ids.
    exploit_path: >
      Rendering raw response text into the status area discloses secrets
      or infrastructure details to the visitor.
    fix: >
      Render fixed generic messages only; never interpolate response body
      or status detail into the DOM. Add a test asserting the rendered error
      contains neither the token nor any fragment of the raw response.
  - id: SEC-14-4
    severity: low
    status: advisory
    where: new newsletter JS (to be written)
    what: >
      Status/confirmation/error messages are an XSS sink if built with
      innerHTML from dynamic values.
    exploit_path: >
      Any server-controlled string reaching innerHTML executes script.
    fix: >
      Use textContent, or reuse the existing escapeHtml helper pattern from
      js/reading-list.js (already tested); extend the component test to cover
      hostile message content.
  - id: SEC-14-5
    severity: low
    status: advisory
    where: endpoint configuration (to be added)
    what: >
      Endpoint URL must be https-only and fixed at config time, validated
      with the same protocol allowlist approach as isValidUrl() in
      js/reading-list.js; never assembled from user input.
    exploit_path: >
      http downgrade or user-influenced endpoint redirects submissions to
      an attacker-controlled collector.
    fix: Enforce https allowlist on the configured endpoint; add a test.
  - id: SEC-14-6
    severity: info
    status: advisory
    where: issue scope + .gitea/workflows/ci.yml
    what: >
      Two residual risks to record: (1) captcha/spam filtering is explicitly
      out of scope, so combined with an openly callable signup endpoint,
      scripted abuse is possible — mitigate with server-side rate limiting
      on the function and note the accepted risk; (2) CI runs only
      `npm test` — given this story touches API tokens, add a gitleaks step
      (fail on any hit) and ensure test fixtures use placeholder token
      values only.
    exploit_path: n/a (risk acknowledgment)
    fix: Document residual risk in README; add secrets scanning to CI.
clean_checks:
  - check: secrets scan
    tool: gitleaks detect --source . --no-git --redact
    result: "no leaks found (exit 0)"
    evidence: run on a byte-exact local reconstruction of main@e24ba89 — 15/15 files matched their git blob SHAs (35.59 KB scanned)
  - check: dependencies
    tool: osv-scanner --recursive .
    result: "No package sources found — zero dependencies and zero lockfiles (verified package.json declares none); no third-party supply-chain surface"
  - check: test suite
    tool: npm test
    result: "30 pass / 0 fail (exit 0)"
authz_trace: >
  No new routes/handlers exist in this static repo; the entire authorization
  burden for signups sits on the external serverless function. It must not
  trust Origin/Referer headers alone — see SEC-14-1 fix (server-held token +
  rate limiting).
positive_observations:
  - js/reading-list.js already establishes escapeHtml(), an http(s) protocol
    allowlist (isValidUrl), rel="noopener noreferrer" hardening, and tests
    rejecting javascript:/ftp: URLs — reuse these patterns.
  - js/mailto.js encodes untrusted input via URLSearchParams.
  - No secrets currently anywhere on main (gitleaks clean).
required_before_implementation:
  - rewrite AC 2/3 per SEC-14-1 (option (a) preferred)
  - add validation-boundary and safe-error criteria per SEC-14-2 / SEC-14-3
note: >
  This closes the design-stage gate for #14. Please re-dispatch security
  review on the implementing PR/diff.
```yaml agent: security-reviewer phase: complete issue: 14 title: "[Story] Newsletter signup (serverless)" review_type: security stage: pre-implementation design review scope: > No PR or feature branch exists for #14 yet (branches: main, feature/5; no open PRs), so this review covers the story's intent and acceptance criteria against the current codebase main@e24ba8916da115cff31d21a36e70c07b9c140b17. A follow-up security pass on the actual diff is still required when the PR opens. verdict: changes-requested blocking_findings: [SEC-14-1] findings: - id: SEC-14-1 severity: high status: blocking where: issue #14 acceptance criteria 2 & 3 + test plan item 2 (design) what: > The site is fully static ("No build step, no backend", README.md). Any "API token from config" that the browser sends is delivered to every visitor no matter where it is stored (js module, JSON, meta tag) — relocating it out of source into a shipped config file changes packaging, not secrecy. Acceptance criterion "without leaking the secret" and the test "token is read from config, never hardcoded" are therefore unsatisfiable as written for confidentiality. exploit_path: > Visitor opens devtools (or curls the config asset), reads the token, then POSTs directly to the serverless endpoint bypassing the form — scripted subscriber-list spam and provider quota/billing abuse. fix: > Pick one BEFORE implementation: (a) preferred — keep the real token server-side only; the serverless function reads it from platform env/secrets at deploy time (matches security-baseline "secrets live in CI/deploy only"), and the page posts nothing secret; or (b) if the provider mandates a client-side key, treat it as publishable-by-design: rename it accordingly, drop the "without leaking" acceptance criterion, document the residual risk, and compensate server-side (rate limiting, origin check). Rewrite AC 2/3 accordingly. - id: SEC-14-2 severity: medium status: advisory where: issue #14 acceptance criteria / test plan (missing requirement) what: > No validation boundary specified. Client-side email checks are UX only; the serverless function must authoritatively validate email syntax, enforce length caps, pin content type, and reject unexpected fields. exploit_path: > Arbitrary oversized or malformed payloads reach the subscription provider if only the form validates. fix: > Add an acceptance criterion + test: server rejects invalid email, over-length input, and unknown fields before processing (security-baseline: validate at the boundary, reject before parse). - id: SEC-14-3 severity: medium status: advisory where: issue #14 acceptance criterion 3 (underspecified) what: > "User-safe error without leaking the secret" needs concrete rules: upstream error bodies may echo tokens, endpoints, or internal ids. exploit_path: > Rendering raw response text into the status area discloses secrets or infrastructure details to the visitor. fix: > Render fixed generic messages only; never interpolate response body or status detail into the DOM. Add a test asserting the rendered error contains neither the token nor any fragment of the raw response. - id: SEC-14-4 severity: low status: advisory where: new newsletter JS (to be written) what: > Status/confirmation/error messages are an XSS sink if built with innerHTML from dynamic values. exploit_path: > Any server-controlled string reaching innerHTML executes script. fix: > Use textContent, or reuse the existing escapeHtml helper pattern from js/reading-list.js (already tested); extend the component test to cover hostile message content. - id: SEC-14-5 severity: low status: advisory where: endpoint configuration (to be added) what: > Endpoint URL must be https-only and fixed at config time, validated with the same protocol allowlist approach as isValidUrl() in js/reading-list.js; never assembled from user input. exploit_path: > http downgrade or user-influenced endpoint redirects submissions to an attacker-controlled collector. fix: Enforce https allowlist on the configured endpoint; add a test. - id: SEC-14-6 severity: info status: advisory where: issue scope + .gitea/workflows/ci.yml what: > Two residual risks to record: (1) captcha/spam filtering is explicitly out of scope, so combined with an openly callable signup endpoint, scripted abuse is possible — mitigate with server-side rate limiting on the function and note the accepted risk; (2) CI runs only `npm test` — given this story touches API tokens, add a gitleaks step (fail on any hit) and ensure test fixtures use placeholder token values only. exploit_path: n/a (risk acknowledgment) fix: Document residual risk in README; add secrets scanning to CI. clean_checks: - check: secrets scan tool: gitleaks detect --source . --no-git --redact result: "no leaks found (exit 0)" evidence: run on a byte-exact local reconstruction of main@e24ba89 — 15/15 files matched their git blob SHAs (35.59 KB scanned) - check: dependencies tool: osv-scanner --recursive . result: "No package sources found — zero dependencies and zero lockfiles (verified package.json declares none); no third-party supply-chain surface" - check: test suite tool: npm test result: "30 pass / 0 fail (exit 0)" authz_trace: > No new routes/handlers exist in this static repo; the entire authorization burden for signups sits on the external serverless function. It must not trust Origin/Referer headers alone — see SEC-14-1 fix (server-held token + rate limiting). positive_observations: - js/reading-list.js already establishes escapeHtml(), an http(s) protocol allowlist (isValidUrl), rel="noopener noreferrer" hardening, and tests rejecting javascript:/ftp: URLs — reuse these patterns. - js/mailto.js encodes untrusted input via URLSearchParams. - No secrets currently anywhere on main (gitleaks clean). required_before_implementation: - rewrite AC 2/3 per SEC-14-1 (option (a) preferred) - add validation-boundary and safe-error criteria per SEC-14-2 / SEC-14-3 note: > This closes the design-stage gate for #14. Please re-dispatch security review on the implementing PR/diff. ```
Member
agent: security
issue: 14
pr: 15
head_branch: feature/14
head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d
phase: started
scope: security review of newsletter signup (serverless POST + API token)
checks_planned:
  - secrets scan (gitleaks)
  - SAST (semgrep)
  - dependency scan (osv-scanner)
  - manual input-boundary & secret-handling trace of new JS/HTML
verdict: pending
```yaml agent: security issue: 14 pr: 15 head_branch: feature/14 head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d phase: started scope: security review of newsletter signup (serverless POST + API token) checks_planned: - secrets scan (gitleaks) - SAST (semgrep) - dependency scan (osv-scanner) - manual input-boundary & secret-handling trace of new JS/HTML verdict: pending ```
Member
agent: reviewer
verdict: request-changes
findings:
  1. file: js/newsletter-config.js
     severity: blocker
     finding: Security issue SEC-14-1 from design review not addressed - API token shipped to client
     fix: >
       As noted in the security review (SEC-14-1), any API token shipped to the client on a static site is inherently public.
       The implementation must either: (a) keep the real token server-side only and have the serverless function read it from
       platform env/secrets at deploy time, or (b) treat the token as publishable-by-design and document the residual risk.
       Currently the implementation ships with an empty placeholder but the design still allows for a secret token to be added,
       which creates a security vulnerability.
```yaml agent: reviewer verdict: request-changes findings: 1. file: js/newsletter-config.js severity: blocker finding: Security issue SEC-14-1 from design review not addressed - API token shipped to client fix: > As noted in the security review (SEC-14-1), any API token shipped to the client on a static site is inherently public. The implementation must either: (a) keep the real token server-side only and have the serverless function read it from platform env/secrets at deploy time, or (b) treat the token as publishable-by-design and document the residual risk. Currently the implementation ships with an empty placeholder but the design still allows for a secret token to be added, which creates a security vulnerability. ```
Member
agent: security
phase: review-started
pr: 15
head_branch: feature/14
head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d
scope:
  - secrets-scan: gitleaks (fail on any hit)
  - dependency-scan: osv-scanner
  - sast: semgrep (only if pre-installed)
  - manual-trace: authz/input-boundary/XSS/secret-exposure on all new files
plan:
  - scan head commit cb9cf70 locally
  - trace newsletter-config.js / newsletter.js / newsletter.html data flow
  - verify user-safe error handling never leaks the token
status: in-progress

Security review of PR #15 starting. A final verdict (approve / request-changes) will follow as a second structured comment on this issue.

```yaml agent: security phase: review-started pr: 15 head_branch: feature/14 head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d scope: - secrets-scan: gitleaks (fail on any hit) - dependency-scan: osv-scanner - sast: semgrep (only if pre-installed) - manual-trace: authz/input-boundary/XSS/secret-exposure on all new files plan: - scan head commit cb9cf70 locally - trace newsletter-config.js / newsletter.js / newsletter.html data flow - verify user-safe error handling never leaks the token status: in-progress ``` Security review of PR #15 starting. A final verdict (`approve` / `request-changes`) will follow as a second structured comment on this issue.
Member
agent: security
phase: started
scope:
  pr: 15
  head_branch: feature/14
  head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d
checks_planned:
  - secrets scan (gitleaks)
  - dependency audit (osv-scanner)
  - authz / input-boundary trace of new endpoints & handlers
  - secret-exposure review of client-shipped config
verdict: pending

Security review of PR #15 is starting. Final structured verdict will follow in a second comment on this issue.

```yaml agent: security phase: started scope: pr: 15 head_branch: feature/14 head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d checks_planned: - secrets scan (gitleaks) - dependency audit (osv-scanner) - authz / input-boundary trace of new endpoints & handlers - secret-exposure review of client-shipped config verdict: pending ``` Security review of PR #15 is starting. Final structured verdict will follow in a second comment on this issue.
Member
agent: security
status: review-started
target: "PR #15 (feature/14)"
issue: "#14"
scope: >
  Security audit of the newsletter signup change set: secrets scan,
  dependency scan, authz/input-boundary trace over the new client-side
  submission path, and secret-handling checks against the repo baseline.
started_at: 2026-08-26T11:20:00Z
final_verdict: pending
```yaml agent: security status: review-started target: "PR #15 (feature/14)" issue: "#14" scope: > Security audit of the newsletter signup change set: secrets scan, dependency scan, authz/input-boundary trace over the new client-side submission path, and secret-handling checks against the repo baseline. started_at: 2026-08-26T11:20:00Z final_verdict: pending ```
Member
agent: security
issue: 14
pr: 15
head_branch: feature/14
head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d
phase: complete
review_type: post-implementation diff review (supersedes started comments #312/#317/#323/#329)
verdict: request-changes
blocking_findings: [SEC-14-R1]
summary: >
  The implementation quality inside its mandated design is good — no secret is
  committed, error paths are genuinely leak-free, and there are no DOM injection
  sinks. But the PR carries forward exactly the architecture the pre-implementation
  security gate (comment 309, SEC-14-1) marked high/blocking: the documented deploy
  procedure places a real bearer token into a client-served JS asset, which makes
  the token public by construction and acceptance criterion 3 ("without leaking
  the secret") unsatisfiable as written. The blocker is cheap to fix NOW because
  NEWSLETTER_ENDPOINT is still a placeholder — once merged and deployed per README,
  the exposure becomes real.
findings:
  - id: SEC-14-R1
    severity: high
    status: blocking
    where: >
      js/newsletter-config.js:7-17 (deploy-time token instruction) +
      README.md:25-28 ("the deployer sets the real token in js/newsletter-config.js")
    what: >
      Client-shipped bearer token. On this static site (no build step, no backend)
      any token placed in newsletter-config.js is served to and readable by every
      visitor; relocating it within shipped assets changes packaging, not secrecy.
      The config comment protects repo confidentiality only ("never committed")
      while leaving runtime confidentiality impossible. With captcha/spam filtering
      explicitly out of scope and no compensating control documented, the token is
      an open door, not a secret. AC 3 holds today only because the token is empty.
    exploit_path: >
      Visitor opens view-source or curls the served js/newsletter-config.js, reads
      the Bearer token, then POSTs directly to the endpoint bypassing all client
      checks — scripted subscriber-list spam and provider quota/billing abuse.
    fix: >
      Pick one before merge: (a) preferred — keep the real token server-side only;
      the serverless function reads it from platform env/secrets and the page posts
      no credential (matches security-baseline "secrets live in CI/deploy only");
      or (b) formally reclassify as publishable-by-design: drop the secrecy framing,
      document residual risk in README, require server-side rate limiting / origin
      allowlist on the function, and rewrite AC 2–3 accordingly. Endpoint is still
      a placeholder (https://example.com/...), so choosing a provider supporting
      server-held keys costs nothing right now.
  - id: SEC-14-R2
    severity: medium
    status: advisory
    where: js/newsletter.js:21-23 (isValidEmail) + issue #14 boundary requirements
    what: >
      Validation boundary is client-side only; the regex and fail-fast checks are
      trivially bypassable via direct API calls, so they are UX, not security.
    exploit_path: >
      Direct POST of arbitrary oversized/malformed payloads reaches the subscription
      provider if only the form validates.
    fix: >
      Specify (and later verify) authoritative validation in the serverless function:
      email syntax, length caps, pinned Content-Type, rejection of unknown fields
      before processing. Track as a required follow-up on the function, not this diff.
  - id: SEC-14-R3
    severity: low
    status: advisory
    where: js/newsletter-config.js:14
    what: >
      Configured endpoint is not protocol-checked; a deployer setting an http://
      value would silently downgrade submissions (design advisory SEC-14-5 unaddressed).
      Not user-influenced in code, hence low.
    exploit_path: >
      http-configured endpoint lets a network observer read submitted addresses
      in transit.
    fix: >
      Enforce https-only on NEWSLETTER_ENDPOINT (config-time assertion or a test),
      mirroring the isValidUrl protocol allowlist pattern already in js/reading-list.js.
  - id: SEC-14-R4
    severity: low
    status: advisory
    where: .gitea/workflows/ci.yml (npm test step only) + README residual-risk note
    what: >
      CI has no secrets-scanning step even though the repo now carries a token-config
      module, and the accepted abuse risk (no captcha/rate limiting) is undocumented.
    exploit_path: n/a (process/residual-risk gap)
    fix: >
      Add a gitleaks step failing on any hit to CI; document the residual signup-abuse
      risk in README (extends design advisory SEC-14-6).
clean_checks:
  - check: secrets scan
    tool: "gitleaks detect --source . --no-git --redact"
    result: "exit 0, 'no leaks found', 54.38 KB scanned"
  - check: dependencies
    tool: "osv-scanner --recursive ."
    result: "'No package sources found' — package.json declares zero dependencies, no lockfile; no third-party supply-chain surface in this diff (exit code reflects no-sources, not a finding)"
  - check: sast
    tool: semgrep
    result: "skipped — binary not installed; gap covered by the manual trace below (per playbook)"
  - check: git history of the token
    tool: "git log -p --follow -- js/newsletter-config.js"
    result: "token introduced as \"\" in its only commit and never non-empty; no secret ever entered history"
  - check: dom/exec sink scan
    tool: "grep innerHTML|outerHTML|document.write|insertAdjacentHTML|eval|new Function over new files"
    result: "no matches"
  - check: test suite
    tool: "npm test"
    result: "47 pass / 0 fail (exit 0) on head cb9cf70"
authz_trace: >
  Zero new routes/handlers/server endpoints in this diff (static site; existing-page
  changes are nav links only). All authorization burden sits on the external
  serverless function, which must not trust Origin/Referer headers alone — see
  SEC-14-R1 fix. Client-side checks (isValidEmail, empty-token guard) are bypassable
  by construction and correctly presented as UX only.
input_boundary_trace: >
  Email flows form -> trim -> shape regex -> JSON.stringify body; no SQL/command/
  template/header sinks exist client-side. Status region is set exclusively via
  textContent (js/newsletter.js:112); response bodies are never parsed or rendered;
  fixed constant messages only — no XSS, no unsafe deserialization, no SSRF (endpoint
  is a compile-time constant, never user-influenced).
positive_observations:
  - "Error hygiene fully implements design advisories SEC-14-3/SEC-14-4: one fixed user-safe message, no status-code/response-body leakage (tests assert absence of token/Bearer/host/status fragments)"
  - "Missing token fails fast with no network call; token travels only in the Authorization header — never URL, body, or messages"
  - "Tests actively assert no secret-shaped literal ships anywhere (tests/newsletter.test.js:108-113)"
merge_conditions:
  - "Resolve SEC-14-R1 via option (a) or (b) and update AC 2–3 + README/config framing to match"
  - "Acknowledge or schedule SEC-14-R2..R4 (advisory items may ride a fast-follow, but R1 gates merge)"
note: >
  Consistent with the pre-implementation gate (comment 309, SEC-14-1) and reviewer
  comment 313. Re-dispatch security on the updated diff; with option (a) chosen this
  should turn approve quickly since every other axis of the change is clean.
```yaml agent: security issue: 14 pr: 15 head_branch: feature/14 head_sha: cb9cf703a870efd31871115a8fdd9f2411067c5d phase: complete review_type: post-implementation diff review (supersedes started comments #312/#317/#323/#329) verdict: request-changes blocking_findings: [SEC-14-R1] summary: > The implementation quality inside its mandated design is good — no secret is committed, error paths are genuinely leak-free, and there are no DOM injection sinks. But the PR carries forward exactly the architecture the pre-implementation security gate (comment 309, SEC-14-1) marked high/blocking: the documented deploy procedure places a real bearer token into a client-served JS asset, which makes the token public by construction and acceptance criterion 3 ("without leaking the secret") unsatisfiable as written. The blocker is cheap to fix NOW because NEWSLETTER_ENDPOINT is still a placeholder — once merged and deployed per README, the exposure becomes real. findings: - id: SEC-14-R1 severity: high status: blocking where: > js/newsletter-config.js:7-17 (deploy-time token instruction) + README.md:25-28 ("the deployer sets the real token in js/newsletter-config.js") what: > Client-shipped bearer token. On this static site (no build step, no backend) any token placed in newsletter-config.js is served to and readable by every visitor; relocating it within shipped assets changes packaging, not secrecy. The config comment protects repo confidentiality only ("never committed") while leaving runtime confidentiality impossible. With captcha/spam filtering explicitly out of scope and no compensating control documented, the token is an open door, not a secret. AC 3 holds today only because the token is empty. exploit_path: > Visitor opens view-source or curls the served js/newsletter-config.js, reads the Bearer token, then POSTs directly to the endpoint bypassing all client checks — scripted subscriber-list spam and provider quota/billing abuse. fix: > Pick one before merge: (a) preferred — keep the real token server-side only; the serverless function reads it from platform env/secrets and the page posts no credential (matches security-baseline "secrets live in CI/deploy only"); or (b) formally reclassify as publishable-by-design: drop the secrecy framing, document residual risk in README, require server-side rate limiting / origin allowlist on the function, and rewrite AC 2–3 accordingly. Endpoint is still a placeholder (https://example.com/...), so choosing a provider supporting server-held keys costs nothing right now. - id: SEC-14-R2 severity: medium status: advisory where: js/newsletter.js:21-23 (isValidEmail) + issue #14 boundary requirements what: > Validation boundary is client-side only; the regex and fail-fast checks are trivially bypassable via direct API calls, so they are UX, not security. exploit_path: > Direct POST of arbitrary oversized/malformed payloads reaches the subscription provider if only the form validates. fix: > Specify (and later verify) authoritative validation in the serverless function: email syntax, length caps, pinned Content-Type, rejection of unknown fields before processing. Track as a required follow-up on the function, not this diff. - id: SEC-14-R3 severity: low status: advisory where: js/newsletter-config.js:14 what: > Configured endpoint is not protocol-checked; a deployer setting an http:// value would silently downgrade submissions (design advisory SEC-14-5 unaddressed). Not user-influenced in code, hence low. exploit_path: > http-configured endpoint lets a network observer read submitted addresses in transit. fix: > Enforce https-only on NEWSLETTER_ENDPOINT (config-time assertion or a test), mirroring the isValidUrl protocol allowlist pattern already in js/reading-list.js. - id: SEC-14-R4 severity: low status: advisory where: .gitea/workflows/ci.yml (npm test step only) + README residual-risk note what: > CI has no secrets-scanning step even though the repo now carries a token-config module, and the accepted abuse risk (no captcha/rate limiting) is undocumented. exploit_path: n/a (process/residual-risk gap) fix: > Add a gitleaks step failing on any hit to CI; document the residual signup-abuse risk in README (extends design advisory SEC-14-6). clean_checks: - check: secrets scan tool: "gitleaks detect --source . --no-git --redact" result: "exit 0, 'no leaks found', 54.38 KB scanned" - check: dependencies tool: "osv-scanner --recursive ." result: "'No package sources found' — package.json declares zero dependencies, no lockfile; no third-party supply-chain surface in this diff (exit code reflects no-sources, not a finding)" - check: sast tool: semgrep result: "skipped — binary not installed; gap covered by the manual trace below (per playbook)" - check: git history of the token tool: "git log -p --follow -- js/newsletter-config.js" result: "token introduced as \"\" in its only commit and never non-empty; no secret ever entered history" - check: dom/exec sink scan tool: "grep innerHTML|outerHTML|document.write|insertAdjacentHTML|eval|new Function over new files" result: "no matches" - check: test suite tool: "npm test" result: "47 pass / 0 fail (exit 0) on head cb9cf70" authz_trace: > Zero new routes/handlers/server endpoints in this diff (static site; existing-page changes are nav links only). All authorization burden sits on the external serverless function, which must not trust Origin/Referer headers alone — see SEC-14-R1 fix. Client-side checks (isValidEmail, empty-token guard) are bypassable by construction and correctly presented as UX only. input_boundary_trace: > Email flows form -> trim -> shape regex -> JSON.stringify body; no SQL/command/ template/header sinks exist client-side. Status region is set exclusively via textContent (js/newsletter.js:112); response bodies are never parsed or rendered; fixed constant messages only — no XSS, no unsafe deserialization, no SSRF (endpoint is a compile-time constant, never user-influenced). positive_observations: - "Error hygiene fully implements design advisories SEC-14-3/SEC-14-4: one fixed user-safe message, no status-code/response-body leakage (tests assert absence of token/Bearer/host/status fragments)" - "Missing token fails fast with no network call; token travels only in the Authorization header — never URL, body, or messages" - "Tests actively assert no secret-shaped literal ships anywhere (tests/newsletter.test.js:108-113)" merge_conditions: - "Resolve SEC-14-R1 via option (a) or (b) and update AC 2–3 + README/config framing to match" - "Acknowledge or schedule SEC-14-R2..R4 (advisory items may ride a fast-follow, but R1 gates merge)" note: > Consistent with the pre-implementation gate (comment 309, SEC-14-1) and reviewer comment 313. Re-dispatch security on the updated diff; with option (a) chosen this should turn approve quickly since every other axis of the change is clean. ```
bot-dispatcher added
loop
2
and removed
loop
1
labels 2026-08-26 11:19:36 +00:00
Member
agent: analyst
issue: 14
phase: start
objective: "Rework issue #14 acceptance criteria to resolve security findings SEC-14-R1..R4"
source: "security review comment 330 — post-implementation diff review of PR #15 (head cb9cf70), verdict request-changes"
decision: "Adopt SEC-14-R1 option (a): token stays server-side in platform env/secrets; the page posts no credential"
plan:
  - "Rewrite AC 2–3 and test plan to drop the client-shipped-token secrecy framing"
  - "Add server-side authoritative validation AC (SEC-14-R2) and https-only endpoint AC (SEC-14-R3)"
  - "Add gitleaks-in-CI and README residual-risk note (SEC-14-R4)"
  - "Edit the issue body, then post phase: rework-complete"
```yaml agent: analyst issue: 14 phase: start objective: "Rework issue #14 acceptance criteria to resolve security findings SEC-14-R1..R4" source: "security review comment 330 — post-implementation diff review of PR #15 (head cb9cf70), verdict request-changes" decision: "Adopt SEC-14-R1 option (a): token stays server-side in platform env/secrets; the page posts no credential" plan: - "Rewrite AC 2–3 and test plan to drop the client-shipped-token secrecy framing" - "Add server-side authoritative validation AC (SEC-14-R2) and https-only endpoint AC (SEC-14-R3)" - "Add gitleaks-in-CI and README residual-risk note (SEC-14-R4)" - "Edit the issue body, then post phase: rework-complete" ```
Member
agent: dispatcher
phase: complete
plan_summary: Dispatch #14: [Story] Newsletter signup (serverless) (platform, security-gated)
```yaml agent: dispatcher phase: complete plan_summary: Dispatch #14: [Story] Newsletter signup (serverless) (platform, security-gated) ```
Member
agent: analyst
issue: 14
phase: rework-complete
verdict: "acceptance criteria reworked; all four findings resolved at the spec level"
decision: "SEC-14-R1 option (a): serverless function reads its API token from platform env/secrets; page posts no credential"
resolutions:
  - finding: SEC-14-R1 (high, blocking)
    resolution: >
      AC 2 now requires the serverless function to read its token from platform
      env/secrets with no credential in the request; the old missing/invalid-token
      AC is replaced by a failed/unavailable-submission user-safe-error AC; AC + test
      plan now assert no secret in any committed/shipped asset and require a gitleaks
      CI step.
  - finding: SEC-14-R2 (medium, advisory)
    resolution: >
      New AC requires authoritative server-side validation (email syntax, length caps,
      pinned Content-Type, reject unknown fields), tracked as a required follow-up on
      the function.
  - finding: SEC-14-R3 (low, advisory)
    resolution: >
      New AC + test enforce https-only on NEWSLETTER_ENDPOINT, mirroring the protocol
      allowlist in js/reading-list.js.
  - finding: SEC-14-R4 (low, advisory)
    resolution: >
      CI gitleaks step (fail on any hit) added to the test plan; residual signup-abuse
      risk documented in README via the out-of-scope section.
next_step: "Re-dispatch security on the updated diff; with option (a) chosen the design should turn approve quickly"
```yaml agent: analyst issue: 14 phase: rework-complete verdict: "acceptance criteria reworked; all four findings resolved at the spec level" decision: "SEC-14-R1 option (a): serverless function reads its API token from platform env/secrets; page posts no credential" resolutions: - finding: SEC-14-R1 (high, blocking) resolution: > AC 2 now requires the serverless function to read its token from platform env/secrets with no credential in the request; the old missing/invalid-token AC is replaced by a failed/unavailable-submission user-safe-error AC; AC + test plan now assert no secret in any committed/shipped asset and require a gitleaks CI step. - finding: SEC-14-R2 (medium, advisory) resolution: > New AC requires authoritative server-side validation (email syntax, length caps, pinned Content-Type, reject unknown fields), tracked as a required follow-up on the function. - finding: SEC-14-R3 (low, advisory) resolution: > New AC + test enforce https-only on NEWSLETTER_ENDPOINT, mirroring the protocol allowlist in js/reading-list.js. - finding: SEC-14-R4 (low, advisory) resolution: > CI gitleaks step (fail on any hit) added to the test plan; residual signup-abuse risk documented in README via the out-of-scope section. next_step: "Re-dispatch security on the updated diff; with option (a) chosen the design should turn approve quickly" ```
Member
agent: dispatcher
phase: complete
plan_summary: Dispatch #14: [Story] Newsletter signup (serverless) (platform, security-gated)
```yaml agent: dispatcher phase: complete plan_summary: Dispatch #14: [Story] Newsletter signup (serverless) (platform, security-gated) ```
Member
agent: implementer
run_id: implementer-14-rework-9c41f2
issue: 14
phase: started
branch: feature/14
loop: 2
evidence: []
cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"}
notes: >
  Rework of PR #15 per security review SEC-14-R1 option (a): drop the
  client-shipped API token entirely. js/newsletter-config.js will carry only
  the non-secret https-only endpoint (validated at config/test time mirroring
  the protocol allowlist in js/reading-list.js); js/newsletter.js will POST
  email-only with no Authorization header and no credential in the request.
  Keeping user-safe error handling (no token/endpoint/status/raw-body leak)
  and the success confirmation, plus a gitleaks CI step and README residual
  abuse-risk note. Server-side authoritative validation is a documented
  follow-up on the function, not this diff.
```yaml agent: implementer run_id: implementer-14-rework-9c41f2 issue: 14 phase: started branch: feature/14 loop: 2 evidence: [] cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"} notes: > Rework of PR #15 per security review SEC-14-R1 option (a): drop the client-shipped API token entirely. js/newsletter-config.js will carry only the non-secret https-only endpoint (validated at config/test time mirroring the protocol allowlist in js/reading-list.js); js/newsletter.js will POST email-only with no Authorization header and no credential in the request. Keeping user-safe error handling (no token/endpoint/status/raw-body leak) and the success confirmation, plus a gitleaks CI step and README residual abuse-risk note. Server-side authoritative validation is a documented follow-up on the function, not this diff. ```
Member
agent: implementer
run_id: implementer-14-125c61be
issue: 14
phase: started
branch: feature/14
loop: 2
evidence: []
cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"}
notes: >
  Reworking PR #15 (feature/14) against the reworked acceptance criteria
  (analyst comment 337, SEC-14-R1 option (a)): the page must POST to the
  configured https-only serverless endpoint with NO credential in the request;
  the serverless function reads its API token from platform env/secrets. Rework
  plan: drop NEWSLETTER_API_TOKEN + Authorization header from the client code,
  enforce https-only on NEWSLETTER_ENDPOINT at config load and in tests (mirror
  the js/reading-list.js protocol allowlist), add a whole-tree secret-shaped
  literal scan test, add a gitleaks CI step that fails on any hit, keep the
  user-safe error (never leaks token/endpoint/status/raw body) and the success
  confirmation, and document the residual signup-abuse risk in README.
```yaml agent: implementer run_id: implementer-14-125c61be issue: 14 phase: started branch: feature/14 loop: 2 evidence: [] cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"} notes: > Reworking PR #15 (feature/14) against the reworked acceptance criteria (analyst comment 337, SEC-14-R1 option (a)): the page must POST to the configured https-only serverless endpoint with NO credential in the request; the serverless function reads its API token from platform env/secrets. Rework plan: drop NEWSLETTER_API_TOKEN + Authorization header from the client code, enforce https-only on NEWSLETTER_ENDPOINT at config load and in tests (mirror the js/reading-list.js protocol allowlist), add a whole-tree secret-shaped literal scan test, add a gitleaks CI step that fails on any hit, keep the user-safe error (never leaks token/endpoint/status/raw body) and the success confirmation, and document the residual signup-abuse risk in README. ```
Member
agent: implementer
run_id: implementer-14-rework-9c41f2
issue: 14
phase: complete
pr: 15
branch: feature/14
head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33
loop: 2
summary: >
  Rework of PR #15 per security review SEC-14-R1 option (a): the client no
  longer carries or sends any credential. js/newsletter-config.js now ships
  only the non-secret NEWSLETTER_ENDPOINT, enforced https-only at config load
  time (validateEndpoint/isHttpsUrl, mirroring the protocol allowlist in
  js/reading-list.js). js/newsletter.js POSTs {email} with no Authorization
  header; failures (non-2xx, network, invalid email) map to one fixed
  user-safe message that never leaks the server-held token, the endpoint, the
  status code, or any raw response body; success shows the confirmation.
  CI gains a gitleaks job that fails on any hit; README documents the
  server-side token, the residual signup-abuse risk (rate limiting / origin
  allowlist on the function), and the authoritative server-side validation
  follow-up (email syntax, length caps, pinned Content-Type, unknown-field
  rejection) as required follow-up on the function, not this diff.

evidence:
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15"
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/86aa3afbbfdeea87cd206f8894d0ce4e64f04f33"
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/86aa3afbbfdeea87cd206f8894d0ce4e64f04f33/tests/newsletter.test.js"

criterion_to_test:
  - criterion: "Signup form renders an email field and submit button on the blog"
    tests:
      - "newsletter page renders an email field and a submit button"
      - "the email field is required and the page loads the wiring module"
      - "every site page links to the newsletter page, and it marks itself current"
  - criterion: "POSTs to the configured https-only serverless endpoint with no credential in the request"
    tests:
      - "submitNewsletterSignup POSTs the email with no Authorization header or credential"
      - "submitNewsletterSignup defaults to the configured endpoint"
      - "the page logic never sends or references a credential"
  - criterion: "Configured endpoint enforced https-only at config/test time (mirrors js/reading-list.js)"
    tests:
      - "isHttpsUrl accepts https and rejects every other scheme"
      - "validateEndpoint throws on a non-https endpoint and accepts an https one"
      - "the configured endpoint is https at config load time (config-time assertion)"
  - criterion: "No API token/secret in any committed source, fixture, or client-served asset; CI gitleaks step fails on any hit"
    tests:
      - "the config module ships the endpoint only — no token export or literal"
      - "no secret-shaped literal ships anywhere in the tree (whole-tree scan)"
      - "CI runs a gitleaks step that fails on any secret hit"
  - criterion: "Failed/unavailable submission surfaces user-safe error never leaking token, endpoint, status, or raw body"
    tests:
      - "a failed submission shows a user-safe error that never leaks token, endpoint, status, or raw body"
      - "a network failure shows the same user-safe error"
      - "an invalid email fails fast with a user-safe error and no network call"
      - "handleNewsletterSubmit surfaces a user-safe error for a failed submission"
  - criterion: "Confirmation message shows after a successful signup"
    tests:
      - "submitNewsletterSignup resolves success with a confirmation message on a 2xx response"
      - "handleNewsletterSubmit shows the confirmation message after a successful signup"
  - criterion: "Serverless function authoritatively validates submissions"
    status: "documented follow-up on the function, not this client-side diff (README); client sends only {email} with pinned Content-Type"

verification:
  - check: "npm test (node --test, node v22.23.2)"
    result: "52/52 pass, 0 fail, 0 skipped"
  - check: "gitleaks detect --source . --no-git --redact (v8.18.4, same flags as the new CI job)"
    result: "no leaks found, exit 0"
  - check: "CI commit status on head 86aa3af"
    result: >
      Both jobs registered and pending "Waiting to run": 'CI / Run tests (pull_request)' and
      'CI / Secret scan (gitleaks) (pull_request)'. This Gitea instance has no Actions runner
      (Actions API returns 404), the same known situation as accepted PRs #6/#9/#11 — no
      failing checks; suite and gitleaks verified green locally.

notes:
  - >
    No API token or secret exists anywhere in the tree or history: the loop-1 token was an
    empty placeholder (never non-empty), and this rework removes the token export entirely.
  - >
    Out of scope respected: no self-hosted backend, no subscriber list management, no spam
    filtering, no captcha. NEWSLETTER_ENDPOINT remains a placeholder (https://example.com/...);
    the deployer sets the real https endpoint at deploy time.
  - >
    PR description carries the criterion -> test table; PR remains open and mergeable
    (mergeable: true) for the security re-review.

cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"}
```yaml agent: implementer run_id: implementer-14-rework-9c41f2 issue: 14 phase: complete pr: 15 branch: feature/14 head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33 loop: 2 summary: > Rework of PR #15 per security review SEC-14-R1 option (a): the client no longer carries or sends any credential. js/newsletter-config.js now ships only the non-secret NEWSLETTER_ENDPOINT, enforced https-only at config load time (validateEndpoint/isHttpsUrl, mirroring the protocol allowlist in js/reading-list.js). js/newsletter.js POSTs {email} with no Authorization header; failures (non-2xx, network, invalid email) map to one fixed user-safe message that never leaks the server-held token, the endpoint, the status code, or any raw response body; success shows the confirmation. CI gains a gitleaks job that fails on any hit; README documents the server-side token, the residual signup-abuse risk (rate limiting / origin allowlist on the function), and the authoritative server-side validation follow-up (email syntax, length caps, pinned Content-Type, unknown-field rejection) as required follow-up on the function, not this diff. evidence: - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/86aa3afbbfdeea87cd206f8894d0ce4e64f04f33" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/86aa3afbbfdeea87cd206f8894d0ce4e64f04f33/tests/newsletter.test.js" criterion_to_test: - criterion: "Signup form renders an email field and submit button on the blog" tests: - "newsletter page renders an email field and a submit button" - "the email field is required and the page loads the wiring module" - "every site page links to the newsletter page, and it marks itself current" - criterion: "POSTs to the configured https-only serverless endpoint with no credential in the request" tests: - "submitNewsletterSignup POSTs the email with no Authorization header or credential" - "submitNewsletterSignup defaults to the configured endpoint" - "the page logic never sends or references a credential" - criterion: "Configured endpoint enforced https-only at config/test time (mirrors js/reading-list.js)" tests: - "isHttpsUrl accepts https and rejects every other scheme" - "validateEndpoint throws on a non-https endpoint and accepts an https one" - "the configured endpoint is https at config load time (config-time assertion)" - criterion: "No API token/secret in any committed source, fixture, or client-served asset; CI gitleaks step fails on any hit" tests: - "the config module ships the endpoint only — no token export or literal" - "no secret-shaped literal ships anywhere in the tree (whole-tree scan)" - "CI runs a gitleaks step that fails on any secret hit" - criterion: "Failed/unavailable submission surfaces user-safe error never leaking token, endpoint, status, or raw body" tests: - "a failed submission shows a user-safe error that never leaks token, endpoint, status, or raw body" - "a network failure shows the same user-safe error" - "an invalid email fails fast with a user-safe error and no network call" - "handleNewsletterSubmit surfaces a user-safe error for a failed submission" - criterion: "Confirmation message shows after a successful signup" tests: - "submitNewsletterSignup resolves success with a confirmation message on a 2xx response" - "handleNewsletterSubmit shows the confirmation message after a successful signup" - criterion: "Serverless function authoritatively validates submissions" status: "documented follow-up on the function, not this client-side diff (README); client sends only {email} with pinned Content-Type" verification: - check: "npm test (node --test, node v22.23.2)" result: "52/52 pass, 0 fail, 0 skipped" - check: "gitleaks detect --source . --no-git --redact (v8.18.4, same flags as the new CI job)" result: "no leaks found, exit 0" - check: "CI commit status on head 86aa3af" result: > Both jobs registered and pending "Waiting to run": 'CI / Run tests (pull_request)' and 'CI / Secret scan (gitleaks) (pull_request)'. This Gitea instance has no Actions runner (Actions API returns 404), the same known situation as accepted PRs #6/#9/#11 — no failing checks; suite and gitleaks verified green locally. notes: - > No API token or secret exists anywhere in the tree or history: the loop-1 token was an empty placeholder (never non-empty), and this rework removes the token export entirely. - > Out of scope respected: no self-hosted backend, no subscriber list management, no spam filtering, no captcha. NEWSLETTER_ENDPOINT remains a placeholder (https://example.com/...); the deployer sets the real https endpoint at deploy time. - > PR description carries the criterion -> test table; PR remains open and mergeable (mergeable: true) for the security re-review. cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"} ```
Member
agent: security
phase: started
task: security-review-of-pr-15
pr: 15
head_branch: feature/14
base_branch: main
head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33
changed_files: 10
additions: 639
deletions: 1
plan:
  - deterministic scans (gitleaks secret scan; osv-scanner dependency audit if available)
  - manual authz/input-boundary trace of new outbound request path (js/newsletter.js, js/newsletter-config.js)
  - leakage review of failure paths (token/endpoint/status/body disclosure, XSS in status region)
  - CI workflow review (.gitea/workflows/ci.yml)
verdict: pending
status: in-progress
```yaml agent: security phase: started task: security-review-of-pr-15 pr: 15 head_branch: feature/14 base_branch: main head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33 changed_files: 10 additions: 639 deletions: 1 plan: - deterministic scans (gitleaks secret scan; osv-scanner dependency audit if available) - manual authz/input-boundary trace of new outbound request path (js/newsletter.js, js/newsletter-config.js) - leakage review of failure paths (token/endpoint/status/body disclosure, XSS in status region) - CI workflow review (.gitea/workflows/ci.yml) verdict: pending status: in-progress ```
Member
agent: tester
phase: start
issue: 14
pr: 15
head_branch: feature/14
head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33
objective: "Independently probe PR #15 for #14 against the reworked acceptance criteria (SEC-14-R1 option (a))"
target: "Fabrika/PersonalBlog — Newsletter signup (serverless)"
scope_files: []
constraints:
  - "never edit the implementer's tests in the same PR"
plan:
  - "Verify signup form renders an email field + submit button on the blog"
  - "Verify submit POSTs to the configured https-only serverless endpoint with NO credential in the request (serverless function reads token from platform env/secrets)"
  - "Verify endpoint is enforced https-only at config/test time (mirrors js/reading-list.js protocol allowlist)"
  - "Verify no API token/secret in any committed source, test fixture, or client-served asset; CI has a gitleaks step failing on any hit"
  - "Verify failed/unavailable submission surfaces a user-safe error that never leaks token/endpoint/status/raw body"
  - "Verify a confirmation message shows after a successful signup"
  - "Verify authoritative server-side validation is documented as a required follow-up on the function (not this client-side diff)"
  - "Reconstruct the tree at the PR head and run npm test + gitleaks independently"
  - "Check CI commit status and post evidence comment with CI links"
```yaml agent: tester phase: start issue: 14 pr: 15 head_branch: feature/14 head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33 objective: "Independently probe PR #15 for #14 against the reworked acceptance criteria (SEC-14-R1 option (a))" target: "Fabrika/PersonalBlog — Newsletter signup (serverless)" scope_files: [] constraints: - "never edit the implementer's tests in the same PR" plan: - "Verify signup form renders an email field + submit button on the blog" - "Verify submit POSTs to the configured https-only serverless endpoint with NO credential in the request (serverless function reads token from platform env/secrets)" - "Verify endpoint is enforced https-only at config/test time (mirrors js/reading-list.js protocol allowlist)" - "Verify no API token/secret in any committed source, test fixture, or client-served asset; CI has a gitleaks step failing on any hit" - "Verify failed/unavailable submission surfaces a user-safe error that never leaks token/endpoint/status/raw body" - "Verify a confirmation message shows after a successful signup" - "Verify authoritative server-side validation is documented as a required follow-up on the function (not this client-side diff)" - "Reconstruct the tree at the PR head and run npm test + gitleaks independently" - "Check CI commit status and post evidence comment with CI links" ```
Member
agent: security
task: security-review-of-pr-15
pr: 15
head_branch: feature/14
base_branch: main
head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33
started_comment: https://git.stevanovic.co.uk/Fabrika/PersonalBlog/issues/14#issuecomment-344
verdict: approve

scans:
  gitleaks_tree:
    command: "gitleaks detect --source . --no-git --redact"
    result: "no leaks found"
    exit_code: 0
  gitleaks_history:
    command: "gitleaks detect --source . --redact --log-opts=--all"
    result: "no leaks found across all 25 branch commits"
    exit_code: 0
    note: >-
      Extra check beyond CI: the PR reworks an earlier design that carried a
      deploy-time token in client-served config — history was scanned to confirm
      no secret was ever committed then removed.
  osv_scanner:
    command: "osv-scanner --recursive ."
    result: "no package sources found — package.json declares zero dependencies and there is no lockfile"
    note: "nothing to audit; not a vulnerability"
  semgrep: "not installed — skipped per playbook, gap covered by manual trace"
  tests:
    command: "npm test @ 86aa3af"
    result: "52/52 pass"
    exit_code: 0

authz_trace:
  new_server_endpoints: none
  authorization_model: >-
    Delegated entirely to the serverless function, which reads its API token
    from platform env/secrets at deploy time. Client ships only the non-secret
    endpoint URL — matches the reworked acceptance criteria in this issue.
  credential_in_request: none
  evidence: >-
    js/newsletter.js POSTs JSON {email} only; tests assert no Authorization
    header of any casing (tests/newsletter.test.js:181-201).

input_boundaries:
  ssrf: "none — outbound URL is the hardcoded NEWSLETTER_ENDPOINT constant, never user-influenced"
  xss: "safe — status region is set via textContent only (js/newsletter.js:108), never innerHTML; no inline handlers in newsletter.html"
  injection: "none — static site, no SQL/template/command paths"
  validation: >-
    Client email check is UX-only and bypassable; README explicitly documents
    authoritative server-side validation (syntax, length caps, pinned
    Content-Type, unknown-field rejection) as a required follow-up on the
    function, per the issue.
  error_leakage: >-
    Fixed user-safe message on invalid email, any non-2xx (400-503 tested) and
    network failure; token/endpoint/status/raw body never surfaced.

findings_non_blocking:
  - id: SEC-14-OBS1
    severity: low
    file: ".gitea/workflows/ci.yml:30"
    what: >-
      CI gitleaks step uses --no-git (tree-only), so a secret committed to
      history and later removed would evade CI.
    exploit_path: >-
      A future commit could delete a leaked secret while it remains retrievable
      from git history, and CI stays green.
    fix: "Drop --no-git or add a history-scan job. Verified clean today (25 commits, 0 findings)."
  - id: SEC-14-OBS2
    severity: low
    file: ".gitea/workflows/ci.yml:22-25"
    what: >-
      gitleaks binary downloaded from GitHub releases over https without
      checksum/signature verification.
    fix: "Pin and verify the release checksum in the workflow."
  - id: SEC-14-OBS3
    severity: info
    file: "js/newsletter-config.js:42"
    what: >-
      NEWSLETTER_ENDPOINT is an example.com placeholder — non-secret and
      https-enforced, but must be pointed at the real function at deploy time.
    fix: "Set the production endpoint at deploy time; assertion fails fast on non-https."
  - id: SEC-14-OBS4
    severity: info
    what: >-
      No captcha/rate limiting on signup — accepted residual abuse risk,
      explicitly out of scope in this issue and documented in README with the
      server-side mitigation requirement.

conclusion: >-
  No blocking findings. Secrets posture is clean in tree and history, no
  credential is sent or shipped, failure paths are leak-safe and XSS-safe,
  and the endpoint is enforced https-only at config load time. Approve;
  observations SEC-14-OBS1/OBS2 are cheap CI hardening follow-ups.
```yaml agent: security task: security-review-of-pr-15 pr: 15 head_branch: feature/14 base_branch: main head_sha: 86aa3afbbfdeea87cd206f8894d0ce4e64f04f33 started_comment: https://git.stevanovic.co.uk/Fabrika/PersonalBlog/issues/14#issuecomment-344 verdict: approve scans: gitleaks_tree: command: "gitleaks detect --source . --no-git --redact" result: "no leaks found" exit_code: 0 gitleaks_history: command: "gitleaks detect --source . --redact --log-opts=--all" result: "no leaks found across all 25 branch commits" exit_code: 0 note: >- Extra check beyond CI: the PR reworks an earlier design that carried a deploy-time token in client-served config — history was scanned to confirm no secret was ever committed then removed. osv_scanner: command: "osv-scanner --recursive ." result: "no package sources found — package.json declares zero dependencies and there is no lockfile" note: "nothing to audit; not a vulnerability" semgrep: "not installed — skipped per playbook, gap covered by manual trace" tests: command: "npm test @ 86aa3af" result: "52/52 pass" exit_code: 0 authz_trace: new_server_endpoints: none authorization_model: >- Delegated entirely to the serverless function, which reads its API token from platform env/secrets at deploy time. Client ships only the non-secret endpoint URL — matches the reworked acceptance criteria in this issue. credential_in_request: none evidence: >- js/newsletter.js POSTs JSON {email} only; tests assert no Authorization header of any casing (tests/newsletter.test.js:181-201). input_boundaries: ssrf: "none — outbound URL is the hardcoded NEWSLETTER_ENDPOINT constant, never user-influenced" xss: "safe — status region is set via textContent only (js/newsletter.js:108), never innerHTML; no inline handlers in newsletter.html" injection: "none — static site, no SQL/template/command paths" validation: >- Client email check is UX-only and bypassable; README explicitly documents authoritative server-side validation (syntax, length caps, pinned Content-Type, unknown-field rejection) as a required follow-up on the function, per the issue. error_leakage: >- Fixed user-safe message on invalid email, any non-2xx (400-503 tested) and network failure; token/endpoint/status/raw body never surfaced. findings_non_blocking: - id: SEC-14-OBS1 severity: low file: ".gitea/workflows/ci.yml:30" what: >- CI gitleaks step uses --no-git (tree-only), so a secret committed to history and later removed would evade CI. exploit_path: >- A future commit could delete a leaked secret while it remains retrievable from git history, and CI stays green. fix: "Drop --no-git or add a history-scan job. Verified clean today (25 commits, 0 findings)." - id: SEC-14-OBS2 severity: low file: ".gitea/workflows/ci.yml:22-25" what: >- gitleaks binary downloaded from GitHub releases over https without checksum/signature verification. fix: "Pin and verify the release checksum in the workflow." - id: SEC-14-OBS3 severity: info file: "js/newsletter-config.js:42" what: >- NEWSLETTER_ENDPOINT is an example.com placeholder — non-secret and https-enforced, but must be pointed at the real function at deploy time. fix: "Set the production endpoint at deploy time; assertion fails fast on non-https." - id: SEC-14-OBS4 severity: info what: >- No captcha/rate limiting on signup — accepted residual abuse risk, explicitly out of scope in this issue and documented in README with the server-side mitigation requirement. conclusion: >- No blocking findings. Secrets posture is clean in tree and history, no credential is sent or shipped, failure paths are leak-safe and XSS-safe, and the endpoint is enforced https-only at config load time. Approve; observations SEC-14-OBS1/OBS2 are cheap CI hardening follow-ups. ```
bot-dispatcher added
kind
story
and removed
kind
story
needs/security-reviewneeds/security-review
labels 2026-08-26 11:31:06 +00:00
Member
agent: reviewer
verdict: approve
```yaml agent: reviewer verdict: approve ```
bot-dispatcher added
status
review
and removed
status
in-progress
kind
story
loop
2
labels 2026-08-26 11:31:58 +00:00
Member
agent: reviewer
verdict: approve
```yaml agent: reviewer verdict: approve ```
Member
agent: reviewer
verdict: approve
```yaml agent: reviewer verdict: approve ```
Member

Security approved; dispatching reviewer.

> Security approved; dispatching reviewer.
Member
agent: implementer
run_id: implementer-14-125c61be
issue: 14
phase: complete
pr: 15
branch: feature/14
head: 32c81d8
loop: 2
evidence:
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15"
  - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/branch/feature/14/tests/newsletter.test.js"
cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"}
summary: >
  Rework of PR #15 against the reworked acceptance criteria (analyst comment
  337, SEC-14-R1 option (a)): the signup POST carries NO credential in the
  request — the serverless function reads its API token from platform
  env/secrets. NEWSLETTER_API_TOKEN and the Authorization header are gone from
  the client; NEWSLETTER_ENDPOINT is the only config value and is enforced
  https-only at config load time via validateEndpoint(), mirroring the
  protocol allowlist in js/reading-list.js (tightened to https). CI gains a
  gitleaks job (detect --source . --no-git --redact) that fails on any hit.
  A whole-tree secret-shaped-literal scan test covers every committed source,
  fixture, and client-served asset. All failure paths (non-2xx, redirects,
  network) map to one fixed user-safe message that never leaks the token, the
  endpoint, the status code, or any raw response body; a confirmation message
  shows after a successful signup. README documents the server-side-only auth,
  the residual signup-abuse risk (rate limiting / origin allowlist on the
  function), and the authoritative server-side validation follow-up.
verification:
  - "npm test on head 32c81d8: 53/53 pass (node v22.23.2, built-in test runner)"
  - "whole-tree secret scan (same patterns the test asserts): clean"
  - "gitleaks v8.18.4 tag exists; CI job registered as 'CI / Secret scan (gitleaks)'"
  - "Actions runner absent on this instance: both checks stay 'Waiting to run' (pending), same as accepted PRs #6/#9/#11; no failing checks"
  - "working tree verified identical to pushed head 32c81d8"
notes: >
  A parallel implementer instance had pushed an equivalent rework (86aa3af) to
  feature/14 after my phase:started comment; I reconciled by adopting that
  base and layering the remaining unique coverage (3xx-redirect failure test,
  no-credential-header/URL assertions) as 32c81d8. PR description carries the
  criterion→test table for all reworked ACs.
```yaml agent: implementer run_id: implementer-14-125c61be issue: 14 phase: complete pr: 15 branch: feature/14 head: 32c81d8 loop: 2 evidence: - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/branch/feature/14/tests/newsletter.test.js" cost: {usd: 0.0, tokens_in: 0, tokens_out: 0, model: "deepseek-v4-flash"} summary: > Rework of PR #15 against the reworked acceptance criteria (analyst comment 337, SEC-14-R1 option (a)): the signup POST carries NO credential in the request — the serverless function reads its API token from platform env/secrets. NEWSLETTER_API_TOKEN and the Authorization header are gone from the client; NEWSLETTER_ENDPOINT is the only config value and is enforced https-only at config load time via validateEndpoint(), mirroring the protocol allowlist in js/reading-list.js (tightened to https). CI gains a gitleaks job (detect --source . --no-git --redact) that fails on any hit. A whole-tree secret-shaped-literal scan test covers every committed source, fixture, and client-served asset. All failure paths (non-2xx, redirects, network) map to one fixed user-safe message that never leaks the token, the endpoint, the status code, or any raw response body; a confirmation message shows after a successful signup. README documents the server-side-only auth, the residual signup-abuse risk (rate limiting / origin allowlist on the function), and the authoritative server-side validation follow-up. verification: - "npm test on head 32c81d8: 53/53 pass (node v22.23.2, built-in test runner)" - "whole-tree secret scan (same patterns the test asserts): clean" - "gitleaks v8.18.4 tag exists; CI job registered as 'CI / Secret scan (gitleaks)'" - "Actions runner absent on this instance: both checks stay 'Waiting to run' (pending), same as accepted PRs #6/#9/#11; no failing checks" - "working tree verified identical to pushed head 32c81d8" notes: > A parallel implementer instance had pushed an equivalent rework (86aa3af) to feature/14 after my phase:started comment; I reconciled by adopting that base and layering the remaining unique coverage (3xx-redirect failure test, no-credential-header/URL assertions) as 32c81d8. PR description carries the criterion→test table for all reworked ACs. ```
Member
agent: reviewer
verdict: approve
```yaml agent: reviewer verdict: approve ```
Member
agent: tester
phase: complete
issue: 14
pr: 15
branch: feature/14
head_sha: 32c81d8b91bd1a4ce6cd0934685ae5f975a9726c
base_sha: e24ba8916da115cff31d21a36e70c07b9c140b17
verdict: pass
target: "Fabrika/PersonalBlog PR #15 (feature/14, head 32c81d8) — Newsletter signup (serverless)"
summary: >
  Independently re-probed the reworked PR head (SEC-14-R1 option (a)) against all 7
  acceptance criteria. The client ships only the non-secret https-only endpoint and
  posts { email } with no credential of any kind; the serverless function reads its
  token from platform env/secrets. I ran the suite, gitleaks (tree + full history),
  and my own 35-check probe script from a fresh checkout — all green. One non-blocking
  test-semantics nuance noted below. No blocker against the acceptance criteria.

acceptance:
  - criterion: "Signup form renders an email field and submit button on the blog"
    status: pass
    evidence: >
      newsletter.html has <form id="newsletter-form">, <input type="email" id="newsletter-email"
      name="email" required>, <button type="submit">Subscribe</button>, and loads
      js/newsletter.js as a module. A Newsletter nav link exists on all four pages,
      with aria-current="page" on the newsletter page.
  - criterion: "POSTs to the configured https-only serverless endpoint with no credential in the request"
    status: pass
    evidence: >
      submitNewsletterSignup issues fetch(endpoint, {method:"POST", headers:{Content-Type:"application/json"},
      body: JSON.stringify({email})}). My probe recorded the call: no Authorization header (case-insensitive),
      no x-api-key/x-auth-token/x-access-token/cookie header, and no token/key/secret in the URL or body.
      js/newsletter-config.js exports NEWSLETTER_ENDPOINT only — no token export, no literal.
  - criterion: "Configured endpoint enforced https-only at config/test time (mirrors js/reading-list.js)"
    status: pass
    evidence: >
      isHttpsUrl() accepts https and rejects http/javascript:/ftp:/protocol-relative/empty; validateEndpoint()
      throws on non-https; NEWSLETTER_ENDPOINT is asserted https at config load time (validateEndpoint runs at
      module bottom). Tests cover all three.
  - criterion: "No API token/secret in committed source, fixture, or client-served asset; CI gitleaks fails on any hit"
    status: pass
    evidence: >
      gitleaks detect --source . --no-git --redact => no leaks found, exit 0 (gitleaks 8.18.4, downloaded + run
      independently). Full-history scan (26 commits) also clean, exit 0. My whole-tree secret-shaped-literal scan
      clean; no credential references in any client-served .js/.html/.css. Token git history: loop-1
      NEWSLETTER_API_TOKEN was "" (empty placeholder, never non-empty) and was removed entirely in the rework.
      .gitea/workflows/ci.yml adds a gitleaks job (gitleaks detect --source . --no-git --redact) that fails on any hit.
  - criterion: "Failed/unavailable submission surfaces a user-safe error that never leaks token, endpoint, status, or raw body"
    status: pass
    evidence: >
      Non-2xx (400/401/403/404/429/500/503), redirects (301/302/307/308), and network throws all map to the single
      fixed NEWSLETTER_ERROR_MESSAGE. My probes asserted the message contains none of a token fragment, the endpoint
      host, or status codes. Status region is written via textContent (no innerHTML), so no XSS sink.
  - criterion: "A confirmation message shows after a successful signup"
    status: pass
    evidence: >
      2xx (200/201/204) resolves {ok:true, message:"Thanks for subscribing!"}; handleNewsletterSubmit surfaces it
      via setStatus(...,"success") into #newsletter-status (role="status", aria-live="polite").
  - criterion: "Serverless function authoritatively validates submissions (email syntax, length caps, pinned Content-Type, unknown-field rejection)"
    status: pass
    evidence: >
      Documented in README as a required follow-up on the function, not this client-side diff. The client sends only
      { email } with a pinned Content-Type: application/json.

tests:
  suite: "node --test (npm test), Node v22.23.2"
  result: "53/53 pass, 0 fail, 0 skipped (my own fresh run at head 32c81d8)"
probes:
  script: "/tmp/probe.mjs (throwaway, outside the repo — implementer's tests untouched)"
  result: "35/35 pass"
  coverage: "form render, POST shape/no-credential, https-only enforcement, whole-tree secret scan, error-path leak checks, success confirmation, README follow-up docs, CI gitleaks step"
integrity:
  - "all 10 changed files at head match the API tree blob SHAs byte-for-byte (git hash-object vs git rev-parse HEAD:path)"
  - "git log --all -- js/newsletter-config.js shows token was only ever \"\" then removed; no secret ever entered history"

ci:
  commit_status: "CI (Run tests + Secret scan (gitleaks))"
  state: pending
  description: "Waiting to run — this Gitea instance has no Actions runner"
  note: >
    Actions API (list_workflows / list_runs) returns 404, i.e. no runner is deployed here — the same known
    situation as accepted PRs #6/#9/#11. No failing checks; the suite and gitleaks are verified green locally
    and independently (results above).
  links:
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/commit/32c81d8b91bd1a4ce6cd0934685ae5f975a9726c"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/32c81d8b91bd1a4ce6cd0934685ae5f975a9726c/.gitea/workflows/ci.yml"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/32c81d8b91bd1a4ce6cd0934685ae5f975a9726c/tests/newsletter.test.js"

findings:
  - severity: info
    note: >
      The new "redirect (3xx) is treated as a failure" test mocks {ok:false, status:301/302/307/308}, but the
      production fetch() uses the default redirect:"follow", so a real 3xx would be followed and the outcome
      would depend on the final response rather than being unconditionally a failure. Not an acceptance-criterion
      issue and no information leak either way (the message is fixed); noted only for test-semantics accuracy.
  - severity: info
    note: >
      NEWSLETTER_ENDPOINT is a placeholder (https://example.com/api/newsletter-subscribers); the feature will not
      POST to a real function until the deployer sets it at deploy time (documented in README). Expected per the
      issue's rollback/config note.

notes:
  - "Diff is 10 files / +664 / -1: README.md, contact.html, css/style.css, index.html, reading.html (nav links),
     plus new js/newsletter-config.js, js/newsletter.js, newsletter.html, tests/newsletter.test.js, and a gitleaks
     job in .gitea/workflows/ci.yml. No changes to unrelated files or pre-existing tests."
  - "Out of scope respected: no self-hosted backend, subscriber list management, spam filtering, or captcha."
  - "No implementer test file was modified by me — the probe lives at /tmp/probe.mjs and is not committed."
```yaml agent: tester phase: complete issue: 14 pr: 15 branch: feature/14 head_sha: 32c81d8b91bd1a4ce6cd0934685ae5f975a9726c base_sha: e24ba8916da115cff31d21a36e70c07b9c140b17 verdict: pass target: "Fabrika/PersonalBlog PR #15 (feature/14, head 32c81d8) — Newsletter signup (serverless)" summary: > Independently re-probed the reworked PR head (SEC-14-R1 option (a)) against all 7 acceptance criteria. The client ships only the non-secret https-only endpoint and posts { email } with no credential of any kind; the serverless function reads its token from platform env/secrets. I ran the suite, gitleaks (tree + full history), and my own 35-check probe script from a fresh checkout — all green. One non-blocking test-semantics nuance noted below. No blocker against the acceptance criteria. acceptance: - criterion: "Signup form renders an email field and submit button on the blog" status: pass evidence: > newsletter.html has <form id="newsletter-form">, <input type="email" id="newsletter-email" name="email" required>, <button type="submit">Subscribe</button>, and loads js/newsletter.js as a module. A Newsletter nav link exists on all four pages, with aria-current="page" on the newsletter page. - criterion: "POSTs to the configured https-only serverless endpoint with no credential in the request" status: pass evidence: > submitNewsletterSignup issues fetch(endpoint, {method:"POST", headers:{Content-Type:"application/json"}, body: JSON.stringify({email})}). My probe recorded the call: no Authorization header (case-insensitive), no x-api-key/x-auth-token/x-access-token/cookie header, and no token/key/secret in the URL or body. js/newsletter-config.js exports NEWSLETTER_ENDPOINT only — no token export, no literal. - criterion: "Configured endpoint enforced https-only at config/test time (mirrors js/reading-list.js)" status: pass evidence: > isHttpsUrl() accepts https and rejects http/javascript:/ftp:/protocol-relative/empty; validateEndpoint() throws on non-https; NEWSLETTER_ENDPOINT is asserted https at config load time (validateEndpoint runs at module bottom). Tests cover all three. - criterion: "No API token/secret in committed source, fixture, or client-served asset; CI gitleaks fails on any hit" status: pass evidence: > gitleaks detect --source . --no-git --redact => no leaks found, exit 0 (gitleaks 8.18.4, downloaded + run independently). Full-history scan (26 commits) also clean, exit 0. My whole-tree secret-shaped-literal scan clean; no credential references in any client-served .js/.html/.css. Token git history: loop-1 NEWSLETTER_API_TOKEN was "" (empty placeholder, never non-empty) and was removed entirely in the rework. .gitea/workflows/ci.yml adds a gitleaks job (gitleaks detect --source . --no-git --redact) that fails on any hit. - criterion: "Failed/unavailable submission surfaces a user-safe error that never leaks token, endpoint, status, or raw body" status: pass evidence: > Non-2xx (400/401/403/404/429/500/503), redirects (301/302/307/308), and network throws all map to the single fixed NEWSLETTER_ERROR_MESSAGE. My probes asserted the message contains none of a token fragment, the endpoint host, or status codes. Status region is written via textContent (no innerHTML), so no XSS sink. - criterion: "A confirmation message shows after a successful signup" status: pass evidence: > 2xx (200/201/204) resolves {ok:true, message:"Thanks for subscribing!"}; handleNewsletterSubmit surfaces it via setStatus(...,"success") into #newsletter-status (role="status", aria-live="polite"). - criterion: "Serverless function authoritatively validates submissions (email syntax, length caps, pinned Content-Type, unknown-field rejection)" status: pass evidence: > Documented in README as a required follow-up on the function, not this client-side diff. The client sends only { email } with a pinned Content-Type: application/json. tests: suite: "node --test (npm test), Node v22.23.2" result: "53/53 pass, 0 fail, 0 skipped (my own fresh run at head 32c81d8)" probes: script: "/tmp/probe.mjs (throwaway, outside the repo — implementer's tests untouched)" result: "35/35 pass" coverage: "form render, POST shape/no-credential, https-only enforcement, whole-tree secret scan, error-path leak checks, success confirmation, README follow-up docs, CI gitleaks step" integrity: - "all 10 changed files at head match the API tree blob SHAs byte-for-byte (git hash-object vs git rev-parse HEAD:path)" - "git log --all -- js/newsletter-config.js shows token was only ever \"\" then removed; no secret ever entered history" ci: commit_status: "CI (Run tests + Secret scan (gitleaks))" state: pending description: "Waiting to run — this Gitea instance has no Actions runner" note: > Actions API (list_workflows / list_runs) returns 404, i.e. no runner is deployed here — the same known situation as accepted PRs #6/#9/#11. No failing checks; the suite and gitleaks are verified green locally and independently (results above). links: - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/commit/32c81d8b91bd1a4ce6cd0934685ae5f975a9726c" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/32c81d8b91bd1a4ce6cd0934685ae5f975a9726c/.gitea/workflows/ci.yml" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/src/commit/32c81d8b91bd1a4ce6cd0934685ae5f975a9726c/tests/newsletter.test.js" findings: - severity: info note: > The new "redirect (3xx) is treated as a failure" test mocks {ok:false, status:301/302/307/308}, but the production fetch() uses the default redirect:"follow", so a real 3xx would be followed and the outcome would depend on the final response rather than being unconditionally a failure. Not an acceptance-criterion issue and no information leak either way (the message is fixed); noted only for test-semantics accuracy. - severity: info note: > NEWSLETTER_ENDPOINT is a placeholder (https://example.com/api/newsletter-subscribers); the feature will not POST to a real function until the deployer sets it at deploy time (documented in README). Expected per the issue's rollback/config note. notes: - "Diff is 10 files / +664 / -1: README.md, contact.html, css/style.css, index.html, reading.html (nav links), plus new js/newsletter-config.js, js/newsletter.js, newsletter.html, tests/newsletter.test.js, and a gitleaks job in .gitea/workflows/ci.yml. No changes to unrelated files or pre-existing tests." - "Out of scope respected: no self-hosted backend, subscriber list management, spam filtering, or captcha." - "No implementer test file was modified by me — the probe lives at /tmp/probe.mjs and is not committed." ```
Member
agent: tester
phase: start
issue: 14
pr: 15
head_branch: feature/14
head_sha: 32c81d8b91bd1a4ce6cd0934685ae5f975a9726c
objective: "Independently probe PR #15 for #14 against the reworked acceptance criteria (SEC-14-R1 option (a))"
target: "Fabrika/PersonalBlog — Newsletter signup (serverless)"
scope_files: []
constraints:
  - "never edit the implementer's tests in the same PR"
plan:
  - "Verify signup form renders an email field + submit button on the blog"
  - "Verify submit POSTs to the configured https-only serverless endpoint with NO credential in the request"
  - "Verify endpoint is enforced https-only at config/test time (mirrors js/reading-list.js)"
  - "Verify no API token/secret in any committed source, test fixture, or client-served asset; CI has a gitleaks step failing on any hit"
  - "Verify failed/unavailable submission surfaces a user-safe error that never leaks token/endpoint/status/raw body"
  - "Verify a confirmation message shows after a successful signup"
  - "Verify authoritative server-side validation is documented as a required follow-up on the function"
  - "Reconstruct the tree at the PR head and run npm test + gitleaks independently"
  - "Check CI commit status and post evidence comment with CI links"
```yaml agent: tester phase: start issue: 14 pr: 15 head_branch: feature/14 head_sha: 32c81d8b91bd1a4ce6cd0934685ae5f975a9726c objective: "Independently probe PR #15 for #14 against the reworked acceptance criteria (SEC-14-R1 option (a))" target: "Fabrika/PersonalBlog — Newsletter signup (serverless)" scope_files: [] constraints: - "never edit the implementer's tests in the same PR" plan: - "Verify signup form renders an email field + submit button on the blog" - "Verify submit POSTs to the configured https-only serverless endpoint with NO credential in the request" - "Verify endpoint is enforced https-only at config/test time (mirrors js/reading-list.js)" - "Verify no API token/secret in any committed source, test fixture, or client-served asset; CI has a gitleaks step failing on any hit" - "Verify failed/unavailable submission surfaces a user-safe error that never leaks token/endpoint/status/raw body" - "Verify a confirmation message shows after a successful signup" - "Verify authoritative server-side validation is documented as a required follow-up on the function" - "Reconstruct the tree at the PR head and run npm test + gitleaks independently" - "Check CI commit status and post evidence comment with CI links" ```
Member
agent: reviewer
verdict: approve
findings: []
```yaml agent: reviewer verdict: approve findings: [] ```
Member
agent: tester
run_id: tester-14-rework-32c81d8
phase: complete
issue: 14
pr: 15
branch: feature/14
head_sha: 32c81d8b91bd1a4ce6cd0934685ae5f975a9726c
verdict: pass
target: "Fabrika/PersonalBlog PR #15 (feature/14, head 32c81d8) — Newsletter signup (serverless), reworked per SEC-14-R1 option (a)"
summary: >
  Independently re-probed the reworked PR head against all 7 acceptance criteria.
  The client now ships only the non-secret https-only endpoint and POSTs {email}
  with no credential in the request; the serverless function reads its token from
  platform env/secrets. 53/53 tests green in my own fresh run; my 33 independent
  boundary probes all pass; gitleaks v8.18.4 finds no leaks in tree or 26-commit
  history. Both CI checks are registered but pending "Waiting to run" (no Actions
  runner on this instance, same as accepted PRs #6/#9/#11).

acceptance:
  - criterion: "Signup form renders an email field and submit button on the blog"
    status: pass
    evidence: >
      newsletter.html contains <form id="newsletter-form">, <input type="email"
      id="newsletter-email" name="email" required autocomplete="email">, and
      <button type="submit">Subscribe</button>, and loads js/newsletter.js as a
      module. Newsletter nav link present on all four pages with aria-current="page"
      on the newsletter page. Verified by direct file inspection at head 32c81d8 and
      by the component tests.

  - criterion: "Submitting POSTs to the configured https-only serverless endpoint with no credential in the request"
    status: pass
    evidence: >
      js/newsletter.js POSTs to NEWSLETTER_ENDPOINT with method POST, headers
      {Content-Type: application/json} only, and body JSON.stringify({email}).
      My independent probe recorded the fetch call and confirmed: no Authorization
      header, no api-key/auth-token/x-access-token/cookie headers, no token/secret/key
      in the URL, and body === {"email":"ada@example.com"}. Config module exports only
      NEWSLETTER_ENDPOINT (no token export). Serverless token is read from platform
      env/secrets at deploy time (documented in README).

  - criterion: "Configured endpoint enforced https-only at config/test time (mirrors js/reading-list.js)"
    status: pass
    evidence: >
      js/newsletter-config.js exports isHttpsUrl() (new URL(...).protocol === "https:")
      and validateEndpoint() which throws on non-https; validateEndpoint(NEWSLETTER_ENDPOINT)
      runs at module load (config-time assertion). This mirrors the protocol allowlist
      pattern of isValidUrl() in js/reading-list.js, tightened to https-only. My probes:
      isHttpsUrl rejects http/javascript:/ftp/garbage/undefined; validateEndpoint throws
      on http and accepts https.

  - criterion: "No API token or secret in any committed source, fixture, or client-served asset; CI gitleaks step fails on any hit"
    status: pass
    evidence: >
      gitleaks v8.18.4 detect --source . --no-git --redact => "no leaks found", exit 0;
      full-history scan (--log-opts=--all, 26 commits) => "no leaks found", exit 0.
      My independent whole-tree secret-shape scan (19 files) => 0 hits. CI
      .gitea/workflows/ci.yml adds a "Secret scan (gitleaks)" job running
      ./gitleaks detect --source . --no-git --redact (gitleaks exits non-zero on any
      finding). Config module ships the endpoint only (no token export or literal); the
      loop-1 token was never non-empty and is removed entirely in this rework.

  - criterion: "A failed or unavailable submission surfaces a user-safe error that never leaks the token, endpoint, status, or raw body"
    status: pass
    evidence: >
      submitNewsletterSignup maps invalid email, any non-2xx (400/401/403/404/429/500/503),
      3xx redirects (301/302/307/308), and network throws to one fixed message
      ("Sorry, the newsletter signup isn't available right now. Please try again later.").
      My probes injected a raw body containing token/host/status/Bearer fragments and
      asserted the surfaced message contains none of them. Status region is written via
      textContent (js/newsletter.js:108), never innerHTML.

  - criterion: "A confirmation message shows after a successful signup"
    status: pass
    evidence: >
      On a 2xx response submitNewsletterSignup resolves {ok:true, message:"Thanks for subscribing!"};
      handleNewsletterSubmit routes it through setStatus(message, "success") into
      #newsletter-status (role="status", aria-live="polite"). Probes confirmed 200/201/204
      and the handler surfaces kind "success".

  - criterion: "Serverless function authoritatively validates submissions (email syntax, length caps, pinned Content-Type, reject unknown fields)"
    status: "pass (documented follow-up on the function, not this client-side diff)"
    evidence: >
      README.md documents this as a required follow-up on the function. The client sends a
      single {email} field with pinned Content-Type: application/json and no extra fields;
      the email shape check is UX-only. Correctly not implemented client-side.

tests:
  suite: "npm test (node --test, node v22.23.2)"
  result: "53/53 pass, 0 fail, 0 skipped"
  note: >
    Ran independently from a fresh clone of feature/14 at head 32c81d8; working tree clean
    and tree sha verified. Matches the implementer's stated 53/53.

probes:
  - >
    Independent throwaway script (in /tmp, not committed; implementer's tests untouched)
    exercised the real js/newsletter.js + js/newsletter-config.js modules: 33/33 boundary/
    property checks pass (https-only endpoint, credential-free POST shape, pinned Content-Type,
    leak-free error on every failure path, success confirmation, email validation edges).
  - >
    Independent whole-tree secret-shape scan (19 files, 11 patterns incl. long opaque strings,
    private keys, gh*/sk-/xox/AKIA/AIza/Bearer/api-key/password) => 0 hits.
  - "gitleaks v8.18.4 tree scan (CI flags) => no leaks found, exit 0"
  - "gitleaks v8.18.4 full-history scan (26 commits) => no leaks found, exit 0"

ci:
  commit_status: pending
  checks:
    - context: "CI / Run tests (pull_request)"
      state: pending
      description: "Waiting to run"
      target_url: "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/0"
    - context: "CI / Secret scan (gitleaks) (pull_request)"
      state: pending
      description: "Waiting to run"
      target_url: "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/1"
  links:
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/0"
    - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/1"
  note: >
    This Gitea instance has no Actions runner: the combined commit status is pending with both
    checks "Waiting to run", and the actions/runs API returns 404 (same as accepted PRs #6/#9/#11).
    No failing checks; suite + gitleaks verified green locally.

constraints:
  - "implementer's tests untouched — no edits to any test file; my probes live outside the repo"

findings:
  - severity: info
    note: >
      CI gitleaks job uses --no-git (tree only), so a secret committed then removed from history
      would evade CI (matches security's SEC-14-OBS1). Verified clean in history today (26 commits,
      0 findings); a history-scan job is cheap hardening.
  - severity: info
    note: >
      NEWSLETTER_ENDPOINT is an example.com placeholder; must be pointed at the real https endpoint
      at deploy time (fails fast on non-https). Non-secret, non-blocking.

notes:
  - >
    Diff is 10 files / +664 / -1: new js/newsletter-config.js, js/newsletter.js, newsletter.html,
    tests/newsletter.test.js; nav-link edits to index/reading/contact; css additions;
    .gitea/workflows/ci.yml gitleaks job; README docs. No existing test file was modified.
  - >
    Out of scope respected: no self-hosted backend, no subscriber list management, no spam filtering,
    no captcha. Server-side validation is correctly a documented follow-up on the function.
```yaml agent: tester run_id: tester-14-rework-32c81d8 phase: complete issue: 14 pr: 15 branch: feature/14 head_sha: 32c81d8b91bd1a4ce6cd0934685ae5f975a9726c verdict: pass target: "Fabrika/PersonalBlog PR #15 (feature/14, head 32c81d8) — Newsletter signup (serverless), reworked per SEC-14-R1 option (a)" summary: > Independently re-probed the reworked PR head against all 7 acceptance criteria. The client now ships only the non-secret https-only endpoint and POSTs {email} with no credential in the request; the serverless function reads its token from platform env/secrets. 53/53 tests green in my own fresh run; my 33 independent boundary probes all pass; gitleaks v8.18.4 finds no leaks in tree or 26-commit history. Both CI checks are registered but pending "Waiting to run" (no Actions runner on this instance, same as accepted PRs #6/#9/#11). acceptance: - criterion: "Signup form renders an email field and submit button on the blog" status: pass evidence: > newsletter.html contains <form id="newsletter-form">, <input type="email" id="newsletter-email" name="email" required autocomplete="email">, and <button type="submit">Subscribe</button>, and loads js/newsletter.js as a module. Newsletter nav link present on all four pages with aria-current="page" on the newsletter page. Verified by direct file inspection at head 32c81d8 and by the component tests. - criterion: "Submitting POSTs to the configured https-only serverless endpoint with no credential in the request" status: pass evidence: > js/newsletter.js POSTs to NEWSLETTER_ENDPOINT with method POST, headers {Content-Type: application/json} only, and body JSON.stringify({email}). My independent probe recorded the fetch call and confirmed: no Authorization header, no api-key/auth-token/x-access-token/cookie headers, no token/secret/key in the URL, and body === {"email":"ada@example.com"}. Config module exports only NEWSLETTER_ENDPOINT (no token export). Serverless token is read from platform env/secrets at deploy time (documented in README). - criterion: "Configured endpoint enforced https-only at config/test time (mirrors js/reading-list.js)" status: pass evidence: > js/newsletter-config.js exports isHttpsUrl() (new URL(...).protocol === "https:") and validateEndpoint() which throws on non-https; validateEndpoint(NEWSLETTER_ENDPOINT) runs at module load (config-time assertion). This mirrors the protocol allowlist pattern of isValidUrl() in js/reading-list.js, tightened to https-only. My probes: isHttpsUrl rejects http/javascript:/ftp/garbage/undefined; validateEndpoint throws on http and accepts https. - criterion: "No API token or secret in any committed source, fixture, or client-served asset; CI gitleaks step fails on any hit" status: pass evidence: > gitleaks v8.18.4 detect --source . --no-git --redact => "no leaks found", exit 0; full-history scan (--log-opts=--all, 26 commits) => "no leaks found", exit 0. My independent whole-tree secret-shape scan (19 files) => 0 hits. CI .gitea/workflows/ci.yml adds a "Secret scan (gitleaks)" job running ./gitleaks detect --source . --no-git --redact (gitleaks exits non-zero on any finding). Config module ships the endpoint only (no token export or literal); the loop-1 token was never non-empty and is removed entirely in this rework. - criterion: "A failed or unavailable submission surfaces a user-safe error that never leaks the token, endpoint, status, or raw body" status: pass evidence: > submitNewsletterSignup maps invalid email, any non-2xx (400/401/403/404/429/500/503), 3xx redirects (301/302/307/308), and network throws to one fixed message ("Sorry, the newsletter signup isn't available right now. Please try again later."). My probes injected a raw body containing token/host/status/Bearer fragments and asserted the surfaced message contains none of them. Status region is written via textContent (js/newsletter.js:108), never innerHTML. - criterion: "A confirmation message shows after a successful signup" status: pass evidence: > On a 2xx response submitNewsletterSignup resolves {ok:true, message:"Thanks for subscribing!"}; handleNewsletterSubmit routes it through setStatus(message, "success") into #newsletter-status (role="status", aria-live="polite"). Probes confirmed 200/201/204 and the handler surfaces kind "success". - criterion: "Serverless function authoritatively validates submissions (email syntax, length caps, pinned Content-Type, reject unknown fields)" status: "pass (documented follow-up on the function, not this client-side diff)" evidence: > README.md documents this as a required follow-up on the function. The client sends a single {email} field with pinned Content-Type: application/json and no extra fields; the email shape check is UX-only. Correctly not implemented client-side. tests: suite: "npm test (node --test, node v22.23.2)" result: "53/53 pass, 0 fail, 0 skipped" note: > Ran independently from a fresh clone of feature/14 at head 32c81d8; working tree clean and tree sha verified. Matches the implementer's stated 53/53. probes: - > Independent throwaway script (in /tmp, not committed; implementer's tests untouched) exercised the real js/newsletter.js + js/newsletter-config.js modules: 33/33 boundary/ property checks pass (https-only endpoint, credential-free POST shape, pinned Content-Type, leak-free error on every failure path, success confirmation, email validation edges). - > Independent whole-tree secret-shape scan (19 files, 11 patterns incl. long opaque strings, private keys, gh*/sk-/xox/AKIA/AIza/Bearer/api-key/password) => 0 hits. - "gitleaks v8.18.4 tree scan (CI flags) => no leaks found, exit 0" - "gitleaks v8.18.4 full-history scan (26 commits) => no leaks found, exit 0" ci: commit_status: pending checks: - context: "CI / Run tests (pull_request)" state: pending description: "Waiting to run" target_url: "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/0" - context: "CI / Secret scan (gitleaks) (pull_request)" state: pending description: "Waiting to run" target_url: "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/1" links: - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/pulls/15" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/0" - "https://git.stevanovic.co.uk/Fabrika/PersonalBlog/actions/runs/21/jobs/1" note: > This Gitea instance has no Actions runner: the combined commit status is pending with both checks "Waiting to run", and the actions/runs API returns 404 (same as accepted PRs #6/#9/#11). No failing checks; suite + gitleaks verified green locally. constraints: - "implementer's tests untouched — no edits to any test file; my probes live outside the repo" findings: - severity: info note: > CI gitleaks job uses --no-git (tree only), so a secret committed then removed from history would evade CI (matches security's SEC-14-OBS1). Verified clean in history today (26 commits, 0 findings); a history-scan job is cheap hardening. - severity: info note: > NEWSLETTER_ENDPOINT is an example.com placeholder; must be pointed at the real https endpoint at deploy time (fails fast on non-https). Non-secret, non-blocking. notes: - > Diff is 10 files / +664 / -1: new js/newsletter-config.js, js/newsletter.js, newsletter.html, tests/newsletter.test.js; nav-link edits to index/reading/contact; css additions; .gitea/workflows/ci.yml gitleaks job; README docs. No existing test file was modified. - > Out of scope respected: no self-hosted backend, no subscriber list management, no spam filtering, no captcha. Server-side validation is correctly a documented follow-up on the function. ```
Author
Owner

merged and closing the issue with it.

merged and closing the issue with it.
kpcto closed this issue 2026-08-26 11:44:31 +00:00
kpcto reopened this issue 2026-08-26 17:34:35 +00:00
kpcto closed this issue 2026-08-26 17:36:24 +00:00
Sign in to join this conversation.