Skip to content

fix(admin): upgrade adagents.json builder and manager from v2 to v3 schema#4415

Merged
bokelley merged 3 commits into
mainfrom
claude/issue-4411-adagents-builder-v3
May 11, 2026
Merged

fix(admin): upgrade adagents.json builder and manager from v2 to v3 schema#4415
bokelley merged 3 commits into
mainfrom
claude/issue-4411-adagents-builder-v3

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Refs #4411

Summary

  • 7 hardcoded v2 schema URLs replaced with v3 across adagents-builder.html (4 occurrences) and AdAgentsManager (suggestion string, createAdAgentsJson, validateProposed). Unit test assertions updated to match.
  • Contact section added (collapsible, file-level): name, email, domain, privacy_policy_url, seller_id, tag_id. Fully round-trips on import.
  • Property features tab added (🏆 Features): inline CSP-safe add/edit form for property_features[] — third-party certification agents (Scope3, TAG, OneTrust) with feature IDs and optional publisher ID. Cards rendered via createElement/textContent (no innerHTML with user data).
  • Per-agent v3 constraint fields in agent modal: exclusive (checkbox), countries (comma-separated alpha-2 with validation feedback for rejected codes), effective_from/effective_until (datetime-local → ISO 8601), encryption_keys (structured X25519 repeater: kid ≤8 chars + base64url x 43–44 chars, strict unpadded regex per RFC 4648 §5, client-side validation), signing_keys (flexible JWK JSON textarea per key).
  • Import paths (importFile, domain-validation flow) fully restore v3 state (contact, property_features, per-agent v3 fields).

Not in scope (deferred by expert consensus)

collections[] and placements[] are the remaining major v3 additions. All three experts (internal-tools-strategist, adtech-product-expert, ad-tech-protocol-expert) agreed these are the largest pieces and should be a follow-up PR. v2 sunset is 2026-08-01, so the TMPX-critical fields (encryption_keys) land now.

Non-breaking

No API contract changes. The builder is a client-side admin tool; exported JSON now validates against the v3 schema. Publishers running v2-generated files are unaffected — the change updates what the builder outputs, not what the server validates against.

Pre-PR expert review sign-offs

Three experts consulted before implementation, two after (on the diff):

Code-reviewer:

  • ✅ XSS in renderPropertyFeatures — fixed (switched to createElement/textContent)
  • ✅ Base64url regex accepted padding (=?) — fixed (strict ^[A-Za-z0-9_-]+$ per RFC 4648)
  • ✅ Dead variable in updateEncKey — removed
  • ✅ Changeset present
  • 📋 Hardcoded #666 colors in new UI (pre-existing pattern in file) — NIT, not fixed to keep diff focused
  • 📋 populateContactForm some(v => v) check — NIT, string fields only, low risk

Internal-tools-strategist:

  • prompt() chain for property features blocked by enterprise CSP — replaced with inline form
  • ✅ Countries field silently drops invalid codes — now alerts and returns with rejected token list
  • 📋 Contact section defaults collapsed — NIT for internal staff audience

Triage-managed PR — opened by the AdCP triage routine for issue #4411. A human reviewer must approve before merge. Do not merge without review.

https://claude.ai/code/session_018ETvNpo7B7a5UjLfiCzaCb


Generated by Claude Code

claude added 2 commits May 11, 2026 20:22
…chema

Fixes hardcoded v2 schema URLs throughout the builder tool and AdAgentsManager,
adds v3-only fields to the builder UI (contact, property_features, per-agent
exclusive/countries/effective_from/effective_until/encryption_keys/signing_keys),
and updates test assertions to match. Collections and placements are deferred
to a follow-up per expert consensus.

Refs #4411

https://claude.ai/code/session_018ETvNpo7B7a5UjLfiCzaCb
…3 upgrade

- Fix XSS in renderPropertyFeatures: switch innerHTML template literals to
  createElement/textContent so imported adagents.json cannot inject HTML
- Replace prompt() chain for property_features CRUD with an inline form
  (CSP-safe, no lost-input risk when user cancels mid-flow)
- Add validation feedback for invalid country codes (alert + return instead
  of silently truncating the list)
- Remove padding character from base64url regex (=? → strict, per RFC 4648 §5
  JWK x values are always unpadded)
- Remove dead getEncryptionKeysFromForm() call in updateEncKey()
- Add changeset for the full v3 upgrade

https://claude.ai/code/session_018ETvNpo7B7a5UjLfiCzaCb
Comment thread server/public/adagents-builder.html Fixed
Comment thread server/public/adagents-builder.html Fixed
…path

Post-expert-review findings on PR #4415:

- renderEncryptionKeysList and renderSigningKeysList interpolated
  user-controlled key fields (kid, x, JWK members) into innerHTML
  template strings. Exploitable via importFile → editAgent: a
  malicious imported adagents.json with `kid: '"><img onerror=...>'`
  would execute in the builder's origin. Switched both renderers to
  createElement + textContent / .value assignment, matching the
  pattern the earlier reviewer applied to renderPropertyFeatures.

- publishers.ts:137 was suggesting v2 schema URL when an adagents.json
  is missing \$schema. Missed v2 emit path in the original PR.

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

Copy link
Copy Markdown
Contributor Author

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

  • XSS in renderEncryptionKeysList / renderSigningKeysList — code-reviewer + security-reviewer both flagged independently. Both renderers interpolated user-controlled key fields (kid, x, JWK members) into innerHTML template strings; exploitable via importFileeditAgent if a publisher imported a malicious adagents.json. Switched both to createElement + textContent/.value, matching the prior fix applied to renderPropertyFeatures.
  • server/src/publishers.ts:137 — missed v2 emit path in the original PR (a fix: suggestion string still pointed at v2). Updated to v3.

E2E coverage (Playwright against the docker-served build, all 4 pass):

  1. Golden path — domain → contact → agent with exclusive/countries (normalized US,ca,GB → [US,CA,GB])/effective window/X25519 encryption_keys → JSON preview asserted to have $schema: …/v3/adagents.json and all new fields.
  2. Invalid country codes (USA, X1) → alert + save blocked.
  3. X25519 x with trailing = → alert + save blocked (regression test for the prior =? regex fix).
  4. XSS regressionstate.agents seeded with payloads '"><img onerror=alert(...)>' in kid/x and '</textarea><script>…</script>' in JWK members; opening the agent for edit no longer fires the dialog, payloads land verbatim as text in inputs/textarea, no injected <img>/<script> nodes.

Admin-merging: the failing Server integration tests check is identically failing on main (commit add005658) — makeWorkOSMock throwing in membership-webhook.test.ts:413 and listCustomersWithOrgIds missing from the stripe-client vitest mock in two registry tests. None of those paths touch anything this PR changes.

@bokelley bokelley merged commit 2598bbe into main May 11, 2026
12 of 13 checks passed
@bokelley bokelley deleted the claude/issue-4411-adagents-builder-v3 branch May 11, 2026 21:09
bokelley added a commit that referenced this pull request May 12, 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 added a commit that referenced this pull request May 12, 2026
* fix(admin): adagents builder v3 validation + UX polish (#4421)

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

* fix(admin): XSS in 4 sibling renderers + 16KB signing-key cap

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>

---------

Co-authored-by: Claude <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 12, 2026
…lder (#4424)

* feat(admin): add Collections and Placements tabs to adagents.json builder (#4420)

Adds v3 parity for collections[] and placements[] surfaces to the
adagents.json builder tool (deferred from PR #4415 by expert consensus).

- Collections tab: 8-field form (collection_id, name, kind, language,
  status, cadence, description) + raw JSON escape hatch for advanced fields
- Placements tab: placement_id, name, description, property_ids checkbox
  list from state.properties, property_tags, placement_tags chip input
- Placement Tags sub-section within Placements tab (same chip+delete
  pattern as existing Signal Tags tab)
- Agent modal: optional Placement constraints section (placement_ids and
  placement_tags checkboxes — overlays on any property-scoped auth type)
- Import (importFile, startManaging) and export (exportFile,
  updateJsonPreview) all round-trip collections, placements, placement_tags
- All card renders use createElement/textContent for XSS safety

No backend or schema changes. server/src/adagents-manager.ts already
emits v3 and is field-agnostic.

https://claude.ai/code/session_01Fkmv4wwAtqD1JoKeaioXgr

* fix(admin): address pre-PR review blockers on collections/placements builder tabs

- Revert spurious adcp_version downgrade in static/schemas/source/index.json
- Strip reserved keys (collection_id, name, etc.) from advanced textarea before
  Object.assign in saveCollectionForm; prevents silent override of validated fields
- Add duplicate-ID guards on saveCollectionForm and savePlacementForm
- Convert state.placementTags from string[] to {tag:{name,description}} map,
  preserving imported metadata on round-trips (import/export/startManaging)
- removePlacementTag cascade-cleans pl.tags[] on every placement (mirrors removeSignalTag)
- createPlacementTag normalises input (.toLowerCase + /[^a-z0-9_]/ → '_')
- savePlacementForm validates property_tags pattern; normalises placement tags on save
- escapeHtml() all user-originated values in innerHTML template literals:
  showPlacementForm (prop.property_id, prop.name), updateAgentPlacementCheckboxes
  (pl.placement_id, pl.name, tag), renderPlacementTags (tag, meta.description)
- Fix hardcoded #666 colours → var(--color-text-muted) in placement checkbox empties
- Expand changeset body with full change description

https://claude.ai/code/session_01Fkmv4wwAtqD1JoKeaioXgr

* fix(admin): renderPlacementTags XSS via attribute-context escape bypass

Post-expert-review finding on PR #4424:

renderPlacementTags built its delete button as
  onclick="removePlacementTag('${escapeHtml(tag)}')"
escapeHtml encodes ' to &#39;, but the browser decodes the HTML
attribute before parsing the JS string literal — so a malicious
placement_tags key like `x'),alert(1)//` breaks out of the JS
context. Reachable via importFile and validateDomain paths.

Rewrote the function with createElement + textContent and a
closure-based onclick (captures `tag` directly), matching the
pattern renderPlacements/renderCollections already use.

E2E (Playwright + Docker): seeded state.placementTags with the
attribute-bypass payload + an HTML-injection payload — no dialog,
no injected <img>, payloads visible as text, delete button still
removes via closure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants