diff --git a/app/web/templates/install.html b/app/web/templates/install.html index d5b6151..768f571 100644 --- a/app/web/templates/install.html +++ b/app/web/templates/install.html @@ -996,7 +996,12 @@ // Role from the Jinja ctx — drives the PAT mint shape below // (analyst tile mints a 1h scope=bootstrap-analyst PAT, admin keeps // the historical 90-day general-purpose PAT). - const ROLE = "{{ role }}"; + // `tojson` produces a JSON-quoted JS string literal, which doubles as + // defense-in-depth: even if `role` ever became attacker-influenced, + // tojson would JSON-escape any embedded quotes/backslashes/script + // sequences. (Today FastAPI's Literal validator already constrains + // `role` to {"analyst","admin"}, so this is belt-and-suspenders.) + const ROLE = {{ role | tojson }}; function copyToClipboard(text) { if (navigator.clipboard && window.isSecureContext) { diff --git a/tests/test_setup_page_roles.py b/tests/test_setup_page_roles.py index 25aef3e..4f9bb08 100644 --- a/tests/test_setup_page_roles.py +++ b/tests/test_setup_page_roles.py @@ -109,6 +109,117 @@ def test_setup_page_admin_js_uses_general_scope(client): assert "expires_in_days" in text # still present in the admin body +def test_setup_page_analyst_clipboard_renders_analyst_layout(client): + """The clipboard text the analyst tile produces must be the analyst layout + (`agnes init` + `agnes catalog`), NOT the admin layout (`claude plugin + marketplace add` + admin-only flow). + + The rendered HTML embeds the role-aware `setup_instructions_lines` text + into a JS array `SETUP_INSTRUCTIONS_TEMPLATE` (see + `_claude_setup_instructions.jinja`); `renderSetupInstructions` substitutes + `{server_url}` / `{token}` into that array at click time. So checking the + embedded array against the served HTML is sufficient — if it carries the + analyst layout, the clipboard payload will too. + + Pinning to the JS array block specifically (not the whole page) avoids + false positives from chrome / preview-mode
renders elsewhere.
+ """
+ import re
+
+ resp = client.get("/setup?role=analyst", follow_redirects=True)
+ assert resp.status_code == 200
+ text = resp.text
+
+ # Locate the JS array that holds the clipboard template body. The partial
+ # emits `var SETUP_INSTRUCTIONS_TEMPLATE = [...].join("\n");` — match
+ # everything between the `[` and the matching `]` so we can scope the
+ # assertions to *just* what gets pasted into Claude Code, ignoring the
+ # preview block that also embeds the lines.
+ match = re.search(
+ r"var\s+SETUP_INSTRUCTIONS_TEMPLATE\s*=\s*\[(.*?)\]\.join\(",
+ text,
+ re.DOTALL,
+ )
+ assert match, "SETUP_INSTRUCTIONS_TEMPLATE array not found in rendered HTML"
+ clipboard_block = match.group(1)
+
+ # Analyst layout markers MUST be in the clipboard block.
+ assert "agnes init" in clipboard_block, (
+ "Analyst clipboard payload missing `agnes init` — is "
+ "compute_default_agent_prompt(role=role) being threaded into "
+ "setup_instructions_lines on the /setup route?"
+ )
+ assert "agnes catalog" in clipboard_block, (
+ "Analyst clipboard payload missing `agnes catalog` smoke verify"
+ )
+
+ # Admin-only markers MUST NOT be in the analyst clipboard block.
+ # If they are, renderSetupInstructions is producing admin layout despite
+ # the analyst tile being selected — the bug this test guards against.
+ # `agnes auth import-token` is the admin login step (analyst uses
+ # `agnes init` which bundles auth + workspace bootstrap).
+ assert "agnes auth import-token" not in clipboard_block, (
+ "Analyst clipboard block contains admin-only `agnes auth import-token` "
+ "— role plumbing is broken; analyst sees admin instructions."
+ )
+ assert "agnes skills" not in clipboard_block, (
+ "Analyst clipboard block contains admin-only skills setup step — "
+ "analyst layout should not include skills management."
+ )
+
+
+def test_setup_page_admin_clipboard_renders_admin_layout(client):
+ """Counterpart to the analyst test — admin tile MUST keep the existing
+ full marketplace/plugins flow byte-equivalent (no regression from Task 4).
+ """
+ import re
+
+ resp = client.get("/setup?role=admin", follow_redirects=True)
+ assert resp.status_code == 200
+ text = resp.text
+
+ match = re.search(
+ r"var\s+SETUP_INSTRUCTIONS_TEMPLATE\s*=\s*\[(.*?)\]\.join\(",
+ text,
+ re.DOTALL,
+ )
+ assert match, "SETUP_INSTRUCTIONS_TEMPLATE array not found in rendered HTML"
+ clipboard_block = match.group(1)
+
+ # Admin layout marker MUST be present. `agnes auth import-token` is the
+ # admin login step (analyst replaces it with `agnes init`); `agnes skills`
+ # is admin-only post-auth setup. Either one anchors the admin layout
+ # without depending on RBAC plugin grants (which the unauthenticated
+ # TestClient won't have).
+ assert "agnes auth import-token" in clipboard_block, (
+ "Admin clipboard payload missing `agnes auth import-token` — "
+ "Task 4 should not have changed admin behavior."
+ )
+ assert "agnes skills" in clipboard_block, (
+ "Admin clipboard payload missing the skills setup step"
+ )
+ # Analyst-only marker MUST NOT appear in admin layout. `agnes init` is
+ # the analyst-only auth + workspace bootstrap; admin uses
+ # `agnes auth import-token` instead.
+ assert "agnes init" not in clipboard_block, (
+ "Admin clipboard block leaked the analyst `agnes init` step"
+ )
+
+
+def test_setup_page_role_is_json_escaped(client):
+ """The `ROLE` JS const must be injected via Jinja `tojson` (defense in
+ depth) — not as a bare `"{{ role }}"` string interpolation. This makes
+ JS string-escaping explicit and removes the dependency on Jinja
+ autoescape semantics for JS contexts.
+ """
+ resp = client.get("/setup?role=analyst", follow_redirects=True)
+ assert resp.status_code == 200
+ text = resp.text
+ # tojson always emits double-quoted JSON: the rendered output is exactly
+ # `const ROLE = "analyst";` (note: no extra space inside the quotes).
+ assert 'const ROLE = "analyst";' in text
+
+
def test_setup_page_js_ternary_keys_bootstrap_to_analyst(client):
"""Mutation-resistant assertion: the `bootstrap-analyst` scope must sit on
the truthy branch of `ROLE === "analyst"`, not on the falsy branch.