Skip to content

fix(security): close P-7 — validate campaignId at api-client boundaries#54

Merged
keyxmakerx merged 2 commits into
mainfrom
claude/fm-sec-chunk-5
May 23, 2026
Merged

fix(security): close P-7 — validate campaignId at api-client boundaries#54
keyxmakerx merged 2 commits into
mainfrom
claude/fm-sec-chunk-5

Conversation

@keyxmakerx

Copy link
Copy Markdown
Owner

Cites: 2026-05-21-core-tenets §T-B1; reports/foundry/2026-05-22-fm-security-audit.md §2 P-7, §4 Chunk 5; dispatches/foundry/FM-SEC-CHUNK-5.md
Security implication: closes P-7 (campaignId interpolation surface). A misconfigured campaignId would silently 404 from Chronicle or — worst case — escape the campaign scope via path injection. Now: validate at the api-client boundary; reject + clear notification if invalid.
Consumer-verified: n/a — operator-controlled setting; this PR adds the validation invariant.
Foundry compatibility: v12 / v13 / v14 — pure JS regex, optional-chaining for ui.notifications accessor.
Mockup: n/a — error notification surface only.

What this changes

File Change
scripts/_settings-validation.mjs (NEW) Exports isValidCampaignId(id) (UUID 8-4-4-4-12 hex regex, case-insensitive) + describeCampaignIdError(id) (returns null for valid; otherwise an operator-actionable error message).
scripts/api-client.mjs Imports describeCampaignIdError. Adds module-private _validateCampaignIdOrThrow(campaignId) helper. Calls it from fetch (line ~201) and uploadMedia (line ~375).
tools/test-campaign-id-validation.mjs (NEW) 16 tests: accept cases (valid UUID, uppercase/mixed-case, v1-v5 version digits), reject cases (empty, whitespace-padded, non-UUID strings, wrong-dash positions, non-hex chars, path-traversal ../, non-string types), error-message coverage, static-source integration checks pinning the validator calls in both fetch and uploadMedia.

Why

The Foundry-side security audit (§2 P-7, §4 Chunk 5) flagged the campaignId interpolation surface. Today's behavior: a misconfigured campaignId (typo / empty / ../) flows into ${baseUrl}/api/v1/campaigns/${campaignId}${path} and either silently 404s from Chronicle (best case) or escapes the campaign scope. After: each Chronicle API call validates campaignId at the boundary; invalid → throw + ui.notifications.error with an actionable "Open Module Settings → Chronicle Sync → Campaign ID" message.

Test plan

  • node --test tools/test-*.mjs passes locally (250 baseline + 16 new = 266/266)
  • node tools/check-package-descriptor.mjs passes
  • Tests cover accept cases (valid v4, uppercase, mixed-case, version-digit variants), reject cases (empty, whitespace, non-UUID, wrong-dash, non-hex, path-traversal, non-string), error-message coverage, integration with fetch/uploadMedia
  • Manual verification in Foundry: configure module with invalid campaignId (e.g., not-a-uuid); verify error notification appears + console.error has the clear message; configure with valid UUID; verify normal operation resumes

Tenet self-check

  • T-B1 security: closes P-7 (SSRF / URL-escape surface)
  • T-B2 plugin isolation: changes stay within chronicle-foundry-module
  • T-B3 production UI: error-notification UX is more actionable than today's silent 404
  • T-B4 dual-audience docs: validator helpers + the test preamble document the threat model

Stop-and-flag

None encountered. Two notes:

  • Validation scope: UUID v1-v5. The dispatch says "check what Chronicle actually emits" — Chronicle emits UUID v4 today. My regex accepts any version digit (1-5) because future Chronicle contracts may shift; the format-only validation is what matters for the URL-escape surface. Flagged for awareness; tightenable if operator wants strict v4.
  • WS path NOT validated (intentional — _doConnect constructs ${wsBase}/ws?token=...&client=foundry-module without campaignId interpolation; campaign association rides on the token, not the URL path). No security gap.

Note on commit history

Two commits via MCP push_files (signing infra failure on chronicle-foundry-module — same as PR #49/50/51/52/53).


🤖 Cites dispatches/foundry/FM-SEC-CHUNK-5.md. Generated by Claude Code.


Generated by Claude Code

… tests (FM-SEC-CHUNK-5 part 1)

New scripts/_settings-validation.mjs exporting isValidCampaignId(id)
and describeCampaignIdError(id). 16 behavioral tests in
tools/test-campaign-id-validation.mjs.

Pair commit follows with the api-client.mjs wiring (import + validate
at fetch/uploadMedia boundaries).

Cites: 2026-05-21-core-tenets §T-B1
       reports/foundry/2026-05-22-fm-security-audit.md §2 P-7, §4 Chunk 5
api-client.mjs::fetch and uploadMedia now validate getSetting('campaignId')
via _validateCampaignIdOrThrow before interpolating into the URL. Pair
commit shipped the validator helper + tests.

On invalid: throws Error + surfaces ui.notifications.error with an
operator-actionable message ("Open Module Settings → Chronicle Sync →
Campaign ID and paste the campaign UUID from Chronicle"). Closes the
SSRF / URL-escape surface AND replaces the today-confusing 404 from
Chronicle with a clear "your config is wrong, here's how to fix" notice.

Test suite: 250 baseline + 16 new = 266/266 pass.

Cites: 2026-05-21-core-tenets §T-B1
       reports/foundry/2026-05-22-fm-security-audit.md §2 P-7, §4 Chunk 5
@keyxmakerx keyxmakerx merged commit a6a2905 into main May 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant