adagents.json builder: legacy quick-add form still emits v2 shape + XSS on imported markup

[bokelley]

Follow-up from PR #4464.

While reviewing #4464, the code-reviewer flagged a second, pre-existing code path in `server/public/adagents-builder.html` around lines 5402-5413 (the legacy "quick-add" form, separate from the main builder state):

1. It reads/writes bare `property_ids` with `tag:`-prefix semantics and no `authorization_type` — same v3 invalidity that fix(admin): adagents.json builder always emits valid v3 authorization_type #4464 fixed in the main flow.
2. It interpolates `agent.url`, `agent.authorized_for`, and property IDs straight into `innerHTML` with no `escapeHtml()`. The path consumes an `adagents.json` fetched by domain — a malicious file can XSS the admin user.

Work

1. Decide whether the quick-add form is still needed (the main builder flow has eclipsed it).
2. If kept: rewrite to use the v3 shape and route the output through `escapeHtml()` consistently.
3. If retired: delete the legacy form.

Why

Pre-existing risk; pulled into focus by the v3 migration. Worth a dedicated PR rather than bundling into the v3-validity fix.

[bokelley]

Triage

Classification: Bug
Bucket(s): security-sensitive, admin/ops tools
Status: ready-for-human

What the experts said:

• security-reviewer: XSS confirmed (medium severity — requires admin to look up a domain with a malicious adagents.json). Key blocker on Option B: escapeHtml() is not safe in value= attribute context inside an innerHTML template — DOM API (createElement + .value) is required. Also flagged broader unescaped reflection in the main validation flow (card_data.name, warning.*, card.errors at lines ~1884–1975 and ~5268–5277) — easier trigger than the legacy form: any admin validating a hostile agent URL hits it.
• internal-tools-strategist: The creator tab is a separate entry point (startCreating()), not just a different code path in the same UI. However, startManaging() already fetches and hydrates a domain's full adagents.json into the v3-clean state machine — no feature gap if the creator tab is retired.

My take: The XSS scope is wider than described: the main validation flow carries the same class of unescaped server/third-party reflection and is the easier-to-trigger path. That fix is required under either option below. @bokelley, your call:

Option A — Retire the creator tab (~250-line deletion):

- startCreating() / updateUIForCreateOrUpdate() / generateAdAgentsJson()
+ // domain-fetch covered by startManaging() → main state machine

Simplifies codebase; eliminates the legacy v2-shape emitter entirely. Confirm no external links point to the creator tab before merging.

Option B — Fix in place (~20-line change):

- newEntry.innerHTML = `...<input ... value="${agent.url}">...`;
+ const urlInput = document.createElement('input');
+ urlInput.className = 'agent-url';
+ urlInput.value = agent.url;   // DOM property — bypasses HTML parser

Plus: add authorization_type to generateAdAgentsJson() emit shape.

Either option must also fix displayValidationResults / displayAgentCardsResults escaping (lines ~1884–1975, ~5268–5277).

Note: security-sensitive label not found in this repo — applying bug only; please create or assign the nearest equivalent manually.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01XcQgEoSpS5WvRQdTy6YUdr

---

Generated by Claude Code

[bokelley]

/triage execute

Verified not superseded — recent work (#4415, #4423, #4424, #4464, #4477) was all v2→v3 schema and authorization_type. The XSS surface and legacy quick-add are still present:

• card.card_data.name unescaped at server/public/adagents-builder.html:1947-1948
• Legacy startCreating() at line 5345
• 61 innerHTML usages in the file

Implementation requirements (per the prior triage):

1. Fix the main validation flow first — it's the easier-trigger path (any admin validating a hostile agent URL hits it). Affected reflections at ~1884-1975 and ~5268-5277: card_data.name, warning.*, card.errors.
2. Do not use escapeHtml() inside value= attribute context in innerHTML templates. Use the DOM API (createElement + .value setter) — security-reviewer flagged the attribute-context bypass explicitly.
3. Retire the legacy quick-add creator tab (startCreating() and its v2-emitting form). startManaging() already fetches and hydrates the v3-clean state machine — no feature gap per internal-tools-strategist.
4. Add a regression test that POSTs a hostile adagents.json (script tag in name, event handler in errors) and asserts the rendered DOM does not contain executable script.

Scope guard: do not refactor the broader 61 innerHTML usages in this PR — only the two reflection clusters and the legacy form retirement. File a follow-up for systematic innerHTML migration.