Skip to content

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

Merged
bokelley merged 3 commits into
mainfrom
claude/issue-4420-adagents-builder-collections-placements
May 12, 2026
Merged

feat(admin): add Collections and Placements tabs to adagents.json builder#4424
bokelley merged 3 commits into
mainfrom
claude/issue-4420-adagents-builder-collections-placements

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Closes #4420

Adds Collections and Placements tabs to the adagents.json builder (server/public/adagents-builder.html), bringing the builder to v3 parity for these two top-level arrays that were deferred from the initial release.

  • Collections tab — CRUD form: collection_id, name, kind, status, cadence, language, description, plus a raw-JSON escape hatch for remaining optional schema fields (genre, content_rating, talent[], etc.). Duplicate-ID guard on save; reserved keys stripped from the advanced textarea to prevent silent override of validated fields.
  • Placements tab — CRUD form: placement_id, name, description, property scope (per-property checkboxes + free-text property tags), and placement tags. Placement tag registry stored as { tag: { name, description } } map matching the placement_tags schema shape; metadata is preserved on import/export round-trips. Tags auto-registered on placement save; cascade-deleted from all placements on tag removal (mirrors removeSignalTag). Pattern validation (^[a-z0-9_]+$) on property tags; placement tags normalised to [a-z0-9_] on save.
  • Agent modal — optional placement_ids / placement_tags overlay fields rendered as live checkboxes on property-scoped agent authorization entries.

Not surfaced in this PR (deliberate scope): collection_ids and format_ids on placement definitions; these can be added in a follow-up once requirements are clearer.

Non-breaking justification

Pure UI addition to a standalone HTML admin tool. No schema changes, no API changes, no protocol changes. The builder is not part of the AdCP protocol surface.

Security

All new innerHTML template literals pipe user-originated values (placement IDs, names, tag strings) through the existing escapeHtml() helper. No new eval, innerHTML without escaping, or dynamic <script> injection.

Pre-PR review

Two expert agents reviewed the diff before this PR was opened:

  • Domain expert (adtech-product-expert): flagged XSS, placement_tags data loss on import, and tag pattern validation — all fixed.
  • Code reviewer (code-reviewer): flagged removePlacementTag cascade gap, Object.assign(col, advanced) override risk, spurious schema file changes, duplicate-ID gap, and tag normalization — all fixed.

Test plan

  • Open builder, add a collection with all fields including advanced JSON; verify round-trip in JSON preview
  • Add a placement with property ID + property tag scope; verify anyOf requirement met
  • Create placement tags via the tag registry UI; verify they appear in agent modal placement-tag checkboxes
  • Delete a tag used by a placement; verify the tag is removed from that placement's tags[] in the JSON preview
  • Import a file with placement_tags metadata; export and verify metadata is preserved verbatim
  • Try saving a collection with a duplicate collection_id; verify alert fires
  • Try saving a collection with {"collection_id":"override"} in the advanced textarea; verify it is silently stripped
  • Try entering an invalid property tag (spaces/uppercase); verify save is blocked with alert

🤖 Triage-managed PR — opened by the AdCP issue-triage agent for issue #4420.
Session: https://claude.ai/code/session_01Fkmv4wwAtqD1JoKeaioXgr


Generated by Claude Code

claude added 2 commits May 11, 2026 21:39
…lder (#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
…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
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>
@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 landed in e1a855e:

  • renderPlacementTags XSS via attribute-context escape bypass — security-reviewer caught this: the delete button was built as onclick="removePlacementTag('${escapeHtml(tag)}')". escapeHtml HTML-encodes ' to &#39;, but the browser decodes the attribute value before parsing the JS string literal — so a placement_tags key like x'),alert(1)// survives encoding and breaks out of the JS context. Reachable via both importFile and validateDomain. Rewrote the function using createElement + textContent and a closure-based onclick (captures tag directly), matching the pattern renderPlacements/renderCollections already use in this PR.

E2E (Playwright + Docker): seeded state.placementTags with "x'),alert('XSSPWN-pt-key')//" (the attribute-bypass payload) and '<img src=x onerror=...>' (the HTML-injection payload). No dialog fires, no injected <img> nodes, payloads visible as text, delete button still removes via closure. Also verified all 4 existing builder e2e tests (golden path + country-code reject + X25519 reject + key XSS regression) still pass.

CI fully green including Server integration tests. Note: the sibling renderers (renderProperties, renderSignals, renderTags, renderSignalTags) were patched in #4423 (just merged) — this PR builds cleanly on that.

@bokelley bokelley merged commit 32d2bb9 into main May 12, 2026
13 checks passed
@bokelley bokelley deleted the claude/issue-4420-adagents-builder-collections-placements 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adagents.json builder: add collections[] and placements[] UI (v3 deferral)

2 participants