From 26be673ea27b14a9fc4049dc376bbd8def6d1f08 Mon Sep 17 00:00:00 2001 From: gus Date: Mon, 11 May 2026 18:53:36 -0300 Subject: [PATCH 1/4] visual-artifacts: PR 4 --interactive copy-only mode Enables the documented interactive layer from the architect spec. v1 scope is copy-only: /plan and /review get an actions card with three buttons (copy as prompt / copy as Markdown / copy as JSON patch). The buttons write to the clipboard via navigator.clipboard.writeText; nothing else. Hard rules locked in CI: - The page's ` would close the surrounding inline -in-payload XSS containment. - 7 new template safety checks (16-19): _js_safe_for_script wired, interactive CSP arm, no document.write / window.open / form submit listeners. - check_absent now ignores comment lines so documenting the forbidden APIs in source comments does not fail the lint. Test counts: - e2e: 266 -> 304 (+38) - template: 31 -> 38 (+7) - Total contract surface: 297 -> 342 --- bin/lib/visual-render.sh | 4 + bin/render-artifact.sh | 189 ++++++++++++++++++++++++-- ci/check-visual-artifact-templates.sh | 39 +++++- ci/e2e-visual-artifacts.sh | 96 ++++++++++++- 4 files changed, 312 insertions(+), 16 deletions(-) diff --git a/bin/lib/visual-render.sh b/bin/lib/visual-render.sh index b37e9da..8282fe3 100644 --- a/bin/lib/visual-render.sh +++ b/bin/lib/visual-render.sh @@ -382,6 +382,10 @@ nano_visual_page_start() { .chip.sev-ok { color: var(--ok); border-color: var(--ok); } .pr-link { color: var(--info); } .unsafe-url { color: var(--bad); font-family: monospace; } + .copy-row { display: flex; flex-wrap: wrap; gap: 8px; align-items: center; margin-bottom: 12px; } + .copy-btn { background: var(--panel-2); color: var(--fg); border: 1px solid var(--line); border-radius: 6px; padding: 6px 12px; cursor: pointer; font: inherit; font-size: 0.9rem; } + .copy-btn:hover, .copy-btn:focus { background: var(--panel); border-color: var(--info); color: var(--info); } + .copy-status { color: var(--muted); font-size: 0.85rem; } diff --git a/bin/render-artifact.sh b/bin/render-artifact.sh index e25ac0a..5f1bb0f 100755 --- a/bin/render-artifact.sh +++ b/bin/render-artifact.sh @@ -10,10 +10,12 @@ # [--interactive] [--out ] # [--manifest-only] # -# PR 1 scope: /plan renderer + manifest + safety locks. Other phases -# exit 1 with a clear "phase not yet supported" message; PR 2 wires -# think/review/security/qa/ship. journal/stack are reserved for PR 3 -# (exit 2). --interactive is reserved for PR 4 (exit 2). +# PR 1 scope: /plan renderer + manifest + safety locks. +# PR 2 wires think/review/security/qa/ship. +# PR 3 wires journal/stack aggregate views. +# PR 4 adds --interactive for copy-only buttons on /plan and /review +# (copy as prompt / Markdown / JSON patch). No filesystem writes, +# no network calls, no form submission. set -e @@ -49,7 +51,10 @@ PR 1+2+3 scope: Flags: --strict require nano_artifact_trust == verified - --interactive reserved for PR 4 (exit 2) + --interactive enable copy-only interactive controls (PR 4 scope: + /plan and /review get "copy as prompt", "copy as Markdown", + and "copy as JSON patch" buttons; no filesystem writes, + no network calls, no form submission) --out write HTML to explicit path under \$NANOSTACK_STORE/visual --manifest-only write manifest only, skip HTML --latest resolve via find-artifact.sh (default if no path) @@ -89,10 +94,7 @@ while [ $# -gt 0 ]; do case "$1" in --latest) USE_LATEST=true ;; --strict) STRICT=true ;; - --interactive) - echo "render-artifact: --interactive is reserved for PR 4 (copy-only interactive mode)" >&2 - exit 2 - ;; + --interactive) INTERACTIVE=true ;; --manifest-only) MANIFEST_ONLY=true ;; --out) shift @@ -487,6 +489,36 @@ HTML printf ' \n' fi printf ' \n' + + # PR 4 interactive: copy buttons that export the plan back to the + # agent in the requested shape. Built from the normalized JSON so + # malformed shapes still produce useful payloads. + if [ "${INTERACTIVE:-false}" = true ]; then + local plan_prompt plan_markdown plan_json_patch + plan_prompt=$(printf '%s' "$norm" | jq -r ' + "Approve the plan and continue.\n" + + "Goal: " + (.summary.goal // "Not recorded") + "\n" + + "Scope: " + (.summary.scope // "Not recorded") + "\n" + + "Planned files:\n - " + ((.summary.planned_files // []) | join("\n - ")) + "\n" + + "Risks:\n - " + ((.summary.risks // []) | join("\n - ")) + ') + plan_markdown=$(printf '%s' "$norm" | jq -r ' + "## Plan summary\n\n" + + "- **Goal:** " + (.summary.goal // "Not recorded") + "\n" + + "- **Scope:** " + (.summary.scope // "Not recorded") + "\n" + + "- **Approval:** " + (.summary.plan_approval // "Not recorded") + "\n\n" + + "### Planned files\n\n" + + ((.summary.planned_files // []) | map("- `" + . + "`") | join("\n")) + ') + plan_json_patch=$(printf '%s' "$norm" | jq -c '{ + action: "plan.approve", + goal: .summary.goal, + scope: .summary.scope, + planned_files: .summary.planned_files, + out_of_scope: .summary.out_of_scope + }') + render_copy_actions "$plan_prompt" "$plan_markdown" "$plan_json_patch" + fi } # Shared helper: find the latest artifact for , surfacing @@ -678,6 +710,40 @@ HTML render_findings_section "$norm" "Review findings" render_context_checkpoint "$norm" + + # PR 4 interactive: review triage payloads. The agent can paste + # the prompt back to drive a re-review or accept the findings + # as-is. + if [ "${INTERACTIVE:-false}" = true ]; then + local review_prompt review_markdown review_json_patch + review_prompt=$(printf '%s' "$norm" | jq -r ' + "Address the review findings before /ship.\n" + + "Summary: " + (.summary.blocking|tostring) + " blocking / " + + (.summary.should_fix|tostring) + " should-fix / " + + (.summary.nitpicks|tostring) + " nitpicks\n" + + "Scope drift: " + (.scope_drift.status // "unknown") + "\n" + + "Now-fix:\n - " + + ((.findings // []) | map(select(.severity == "blocking" or .severity == "should_fix")) | map(.description) | join("\n - ")) + ') + review_markdown=$(printf '%s' "$norm" | jq -r ' + "## Review findings\n\n" + + "| Severity | Count |\n|----|----|\n" + + "| Blocking | " + (.summary.blocking|tostring) + " |\n" + + "| Should fix | " + (.summary.should_fix|tostring) + " |\n" + + "| Nitpicks | " + (.summary.nitpicks|tostring) + " |\n" + + "| Positive | " + (.summary.positive|tostring) + " |\n\n" + + "### Now-fix\n\n" + + ((.findings // []) | map(select(.severity == "blocking" or .severity == "should_fix")) + | map("- **" + (.severity // "?") + "**: " + (.description // "")) | join("\n")) + ') + review_json_patch=$(printf '%s' "$norm" | jq -c '{ + action: "review.triage", + blocking_ids: ((.findings // []) | map(select(.severity == "blocking") | .id)), + should_fix_ids: ((.findings // []) | map(select(.severity == "should_fix") | .id)), + scope_drift: .scope_drift.status + }') + render_copy_actions "$review_prompt" "$review_markdown" "$review_json_patch" + fi } render_security_body() { @@ -897,6 +963,105 @@ render_findings_section() { printf ' \n' } +# ─── Copy-only interactive controls (PR 4) ────────────────── +# +# Emits an actions card with three buttons (copy as prompt / copy as +# Markdown / copy as JSON patch). Each button copies a pre-computed +# payload to the clipboard. The script uses only +# navigator.clipboard.writeText; no fetch, no XHR, no localStorage, +# no form submission. CI lockes those forbidden APIs. +# +# Payloads are passed as JSON-encoded strings (via nano_json_string) +# so embedded quotes, newlines, and control characters stay safe. +# Each payload is also rendered as a 
...
block +# so the user can read and copy manually if the browser blocks +# clipboard access (e.g. file:// without explicit permission). +render_copy_actions() { + local prompt_payload="$1" + local markdown_payload="$2" + local json_payload="$3" + if [ "${INTERACTIVE:-false}" != true ]; then + return 0 + fi + + # Encode each payload as a JSON string literal so the inline JS + # can read it via JSON.parse. nano_json_string emits the full + # quoted form including the outer ". Critically: when embedding a + # JSON string INSIDE , any `` regardless of JS context. Any + # following content executes as a new + +HTML +} + # Shared helper for the context checkpoint card. Every core phase has # the same checkpoint shape; centralizing keeps the visual identity # consistent. @@ -1563,7 +1728,9 @@ CREATED_AT=$(date -u +%Y-%m-%dT%H:%M:%SZ) # survives outside. if [ "$MANIFEST_ONLY" != true ]; then { - nano_visual_page_start "$PHASE" "$TRUST" "static" + csp_mode="static" + [ "$INTERACTIVE" = true ] && csp_mode="interactive" + nano_visual_page_start "$PHASE" "$TRUST" "$csp_mode" if [ "$SCHEMA_OK" = false ]; then printf '
%s
\n' \ @@ -1618,7 +1785,7 @@ jq -n \ --arg phase "$PHASE" \ --argjson custom_phase false \ --arg format "html" \ - --argjson interactive false \ + --argjson interactive "$($INTERACTIVE && echo true || echo false)" \ --argjson strict "$($STRICT && echo true || echo false)" \ --argjson source_artifacts "$SOURCE_ARTIFACTS_JSON" \ --arg output_path "$HTML_PATH" \ diff --git a/ci/check-visual-artifact-templates.sh b/ci/check-visual-artifact-templates.sh index 6c94c7e..ee0564d 100755 --- a/ci/check-visual-artifact-templates.sh +++ b/ci/check-visual-artifact-templates.sh @@ -38,12 +38,18 @@ check_absent() { local name="$1"; local pattern="$2"; shift 2 local files=("$@") # `-e -- ` so a pattern starting with `-` (e.g. `--no-session-sync`) - # is not interpreted as a grep flag. - if grep -nE -e "$pattern" -- "${files[@]}" >/dev/null 2>&1; then + # is not interpreted as a grep flag. Exclude comment lines (PR 4): + # documenting the forbidden APIs in comments is fine, only actual + # source uses fail the lint. + local hits + hits=$(grep -nE -e "$pattern" -- "${files[@]}" 2>/dev/null \ + | grep -v '^[^:]*:[0-9]*:[[:space:]]*#' \ + || true) + if [ -n "$hits" ]; then FAIL=$((FAIL+1)) printf " ${RED}FAIL${NC} %s\n" "$name" printf " ${DIM}pattern: %s${NC}\n" "$pattern" - grep -nE -e "$pattern" -- "${files[@]}" | sed 's/^/ /' || true + printf '%s\n' "$hits" | sed 's/^/ /' else PASS=$((PASS+1)) printf " ${GREEN}OK${NC} %s\n" "$name" @@ -201,6 +207,33 @@ check_absent "no SVG for prompt" "$HTML" 'data-testid="copy-prompt-pre"' +assert_contains "interactive plan uses navigator.clipboard.writeText" "$HTML" 'navigator.clipboard.writeText' +# Capture the interactive manifest BEFORE the static render so a +# later mtime sort cannot pick the wrong one. +MFST_I=$(ls -t "$NANOSTACK_STORE/visual/manifests/"*plan*.manifest.json | head -1) +assert_true "interactive manifest records interactive: true" \ + sh -c "[ \"\$(jq -r .interactive '$MFST_I')\" = 'true' ]" +# Without --interactive, NO copy section. +HTML_S=$(cd "$PROJ" && "$REPO/bin/render-artifact.sh" plan --latest) +assert_not_contains "static plan has NO copy-actions" "$HTML_S" 'data-testid="copy-actions"' +assert_not_contains "static CSP does NOT allow script-src" "$HTML_S" "script-src 'unsafe-inline'" + +# ─── Cell 23e: --interactive CSP allows inline script (PR 4) ─ +printf "\n ${DIM}Cell 23e: interactive CSP (PR 4)${NC}\n" +PROJ="$TMP_ROOT/cell23e" +setup_project "$PROJ" +export NANOSTACK_STORE="$PROJ/.nanostack" +mkdir -p "$NANOSTACK_STORE" +(cd "$PROJ" && save_valid_plan "$NANOSTACK_STORE") +HTML=$(cd "$PROJ" && "$REPO/bin/render-artifact.sh" plan --latest --interactive) +assert_contains "interactive CSP includes script-src 'unsafe-inline'" "$HTML" "script-src 'unsafe-inline'" +assert_contains "interactive CSP still locks default-src 'none'" "$HTML" "default-src 'none'" +assert_contains "interactive CSP still locks form-action 'none'" "$HTML" "form-action 'none'" +# Static still has the tighter CSP. +HTML_S=$(cd "$PROJ" && "$REPO/bin/render-artifact.sh" plan --latest) +assert_not_contains "static CSP has NO script-src directive" "$HTML_S" "script-src" + +# ─── Cell 23f: interactive output is forbidden-API free (PR 4) ─ +printf "\n ${DIM}Cell 23f: forbidden APIs absent from interactive output (PR 4)${NC}\n" +PROJ="$TMP_ROOT/cell23f" +setup_project "$PROJ" +export NANOSTACK_STORE="$PROJ/.nanostack" +mkdir -p "$NANOSTACK_STORE" +(cd "$PROJ" && save_valid_plan "$NANOSTACK_STORE") +(cd "$PROJ" && save_valid_review "$NANOSTACK_STORE") +HTML_P=$(cd "$PROJ" && "$REPO/bin/render-artifact.sh" plan --latest --interactive) +HTML_R=$(cd "$PROJ" && "$REPO/bin/render-artifact.sh" review --latest --interactive) +for h in "$HTML_P" "$HTML_R"; do + assert_not_contains "no fetch( in $(basename $h)" "$h" "fetch(" + assert_not_contains "no XMLHttpRequest in $(basename $h)" "$h" "XMLHttpRequest" + assert_not_contains "no sendBeacon in $(basename $h)" "$h" "sendBeacon" + assert_not_contains "no localStorage in $(basename $h)" "$h" "localStorage" + assert_not_contains "no sessionStorage in $(basename $h)" "$h" "sessionStorage" + assert_not_contains "no document.cookie in $(basename $h)" "$h" "document.cookie" + assert_not_contains "no eval( in $(basename $h)" "$h" "eval(" + assert_not_contains "no new Function in $(basename $h)" "$h" "new Function" + assert_not_contains "no
in payload does NOT break out of inline script (PR 4 XSS) ─ +printf "\n ${DIM}Cell 23h: payload XSS containment (PR 4)${NC}\n" +PROJ="$TMP_ROOT/cell23h" +setup_project "$PROJ" +export NANOSTACK_STORE="$PROJ/.nanostack" +mkdir -p "$NANOSTACK_STORE" +(cd "$PROJ" && NANOSTACK_STORE="$NANOSTACK_STORE" "$REPO/bin/save-artifact.sh" plan '{ + "phase":"plan", + "summary":{"goal":"\"","scope":"s","planned_files":[""],"plan_approval":"manual"}, + "context_checkpoint":{"summary":"x","key_files":[],"decisions_made":[],"open_questions":[]} +}' >/dev/null) +HTML=$(cd "$PROJ" && "$REPO/bin/render-artifact.sh" plan --latest --interactive) +# The malicious sequence must NOT appear as literal `` regardless of JS context. Any - # following content executes as a new , several HTML parser states + # can swallow or re-interpret the script body. Codex PR 4 pass 3 + # caught that escaping only `` sequence + # able to put the parser into the "script data double escaped" + # state, which then eats the legitimate closing ``. + # + # The fix: escape EVERY `<` as `<` in the script-embedded + # payload. JSON.parse reads `<` as the literal `<`, so the + # clipboard content the user pastes is unchanged. The HTML parser + # never sees a `<` inside the script body and stays in the + # standard "script data" state until the real closing tag. _js_safe_for_script() { - sed 's|