Skip to content

fix(admin): adagents.json builder v3 validation + UX polish#4423

Merged
bokelley merged 2 commits into
mainfrom
claude/issue-4421-adagents-builder-v3-validation-polish
May 12, 2026
Merged

fix(admin): adagents.json builder v3 validation + UX polish#4423
bokelley merged 2 commits into
mainfrom
claude/issue-4421-adagents-builder-v3-validation-polish

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Refs #4421

Six validation and UX fixes from expert review of PR #4415 (v2→v3 builder upgrade). All items were non-blocking at merge time; this PR cleans them up.

Changes

Item 1 — importFile validation bypass + renderAgents self-XSS

  • importFile() now calls normalizeImportedData() before loading state: uppercases and deduplicates country codes, warns about malformed encryption_keys.x values, lowercases contact.domain. Returns a summary alert listing any coerced entries (warn-and-import, not hard-reject).
  • Reference-only stubs (authoritative_location with no authorized_agents) now refuse to load with a helpful message pointing to the actual file URL.
  • Structural sanity check before loading (authorized_agents or authoritative_location must be present).
  • renderAgents() template literal: wrapped agent.url, agent.authorized_for, agent.delegation_type, tagNames, propNames in the existing escapeHtml() helper to close a self-XSS vector in imported file rendering.

Item 2 — Countries: dedupe + empty-token tolerance (saveAgent())

  • Added .filter(Boolean) to remove empty tokens from trailing/double commas — previously aborted save with confusing "invalid code" error.
  • Added [...new Set(...)] deduplication — schema uniqueItems: true would reject duplicates on strict validators.

Item 3 — Half-filled encryption key rows silently dropped (saveAgent())

  • Before filter(k => k.kid && k.x), now checks for rows with one field filled but not the other and alerts with 1-based row index.

Item 4 — Signing keys: silent JSON parse failure (saveAgent())

  • Iterates [data-sig-index] textareas directly; alerts with 1-based row indices for any rows containing invalid JSON before getSigningKeysFromForm() silently drops them.

Item 5 — contact.domain lowercase normalization (getContactFromForm())

  • .toLowerCase() applied on save. Schema pattern requires lowercase host syntax.

Item 6 — Test coverage for v3 fields (adagents-manager.test.ts)

  • createAdAgentsJson: new round-trip test for exclusive, countries, effective_from/until, signing_keys.
  • validateProposed: 4 new tests — valid v3 fields accepted, invalid country codes rejected, effective_until < effective_from rejected, malformed signing_keys (missing kid) rejected.

Non-breaking justification

All changes are in a client-side admin HTML tool (server/public/) and a server unit test file. No protocol schemas, no API contracts, no published spec surfaces touched. The worst failure mode is an additional alert box.

Pre-PR review

  • code-reviewer: approved — one blocker found and fixed (reference-only file import data loss); renderAgents escaping correct; countries dedup logic correct; signing-key parse-check logic correct; test as any casts appropriate.
  • internal-tools-strategist: approved — all 6 items correctly implemented; item-by-item UX assessment confirms alert-on-save pattern is right for quarterly-use admin tool; normalizeImportedData mutation on fresh parse result is safe.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See #3121
for context.

Session: https://claude.ai/code/session_01RunY4jCVuW6pABsJZsBiXk


Generated by Claude Code

Six validation and UX fixes from expert review of #4415.
Closes silent data loss on enc key rows and signing key JSON errors,
adds import-time validation pass, escapes innerHTML in renderAgents,
and adds v3 field test coverage in adagents-manager.test.ts.

https://claude.ai/code/session_01RunY4jCVuW6pABsJZsBiXk
@bokelley bokelley added the claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage. label May 11, 2026
Post-expert-review findings on PR #4423:

- renderProperties/renderSignals/renderTags/renderSignalTags
  interpolated state into innerHTML template strings unescaped.
  Same class as the bug PR #4415 fixed in renderAgents — security
  reviewer pointed out the four siblings also need escapeHtml, and
  the new importFile validation pass routes attacker-controlled
  state straight into them. Wrapped every state interpolation in
  escapeHtml; rewrote bulk-tag-select dropdown via createElement +
  textContent + .value (option value attribute is the most
  dangerous sink for tag names).

- renderSigningKeysList: added textarea.maxLength = 16384 to back
  the "signing-key size cap" claim from the PR description (the
  in-PR JSON parse feedback shipped, but the size cap was missing).

E2E (Playwright + Docker, all 5 pass): golden path, country-code
reject, X25519 reject, encryption/signing key XSS regression, and
new property/signal/tag/signal-tag XSS regression seeding malicious
state and asserting no dialog fires + no injected <img> nodes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley marked this pull request as ready for review May 12, 2026 01:45
@bokelley

Copy link
Copy Markdown
Contributor Author

Post-expert-review fix-up landed in b069363:

  • XSS in 4 sibling renderers — security-reviewer flagged that renderProperties, renderSignals, renderTags, renderSignalTags still interpolated state via innerHTML template strings unescaped, same class as the bug PR fix(admin): upgrade adagents.json builder and manager from v2 to v3 schema #4415 fixed in renderAgents. The new importFile validation pass routes attacker-controlled state straight into them. Wrapped every state interpolation in escapeHtml; rewrote bulk-tag-select dropdown via createElement + textContent + .value (option value is the most dangerous attribute sink for tag names).
  • Signing-key 16 KB cap — PR description claimed this; was missing. Added textarea.maxLength = 16384 in renderSigningKeysList.

E2E (Playwright + Docker, 5/5 pass): golden path + country-code reject + X25519 =-padding reject + encryption/signing key XSS regression + new property/signal/tag/signal-tag XSS regression seeding malicious state and asserting no dialog fires + no injected <img> nodes.

CI fully green this round including Server integration tests (the upstream flake from yesterday cleared).

@bokelley bokelley merged commit 322bc1c into main May 12, 2026
13 checks passed
@bokelley bokelley deleted the claude/issue-4421-adagents-builder-v3-validation-polish branch May 12, 2026 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants