Skip to content

Commit 32dfa39

Browse files
authored
visual-artifacts v1: PR 2 core phase renderers (think/review/security/qa/ship) (#218)
* visual-artifacts: PR 2 core phase renderers (think/review/security/qa/ship) Wires renderers for the remaining core phases so /think, /review, /security, /qa, and /ship artifacts can each be inspected as a static HTML view. Same trust + path-safety contract as PR 1. Shared helpers in bin/lib/visual-render.sh: - nano_visual_normalize_artifact: jq-based coercion that turns a legacy or malformed artifact into a predictable shape (summary, context_checkpoint, scope_drift, findings, conflicts as objects / arrays). Every renderer reads the normalized form. - nano_visual_severity_class: maps blocking/critical/high/etc to a CSS class so review/security/qa agree on color and styling. - nano_visual_safe_pr_url: allowlist for ship pr_url. Only https://github.com/* renders as a clickable link; every other URL renders as escaped text with an explicit "host not in allowlist" note. - nano_visual_safe_screenshot_path: stricter allowlist for future QA screenshot rendering (PR 3 may wire this). Body renderers in bin/render-artifact.sh: - render_think_body: value proposition, scope mode chip, narrowest wedge, key risk, premise validation, archetype card with optional example reference, out-of-scope list. - render_review_body: 4-counter summary (blocking / should fix / nitpicks / positive), scope drift status chip with out-of-scope and missing file lists, severity-styled findings. - render_security_body: 5-counter summary (critical / high / medium / low / total), findings with category chips (OWASP A0n / STRIDE), proof_of_concept and reproduce blocks wrapped in <details><pre> so multi-line escaped content stays readable. - render_qa_body: mode / status chips, WTF likelihood, 5-counter test/bug breakdown, findings with reproduce + root_cause + fixed flag. - render_ship_body: report_only mode renders a short report card (no release-packet styling); normal mode renders PR title / number / URL / status / CI passed. Unsafe PR URLs are rendered as text with data-testid="unsafe-pr-url" so callers can audit. - render_findings_section: shared helper for review/security/qa. - render_context_checkpoint: shared helper for the trailing card. CI extensions: - 8 new e2e cells (think, review, security, qa, ship normal, ship report_only, ship malicious URL, XSS across all 5 phases). - 5 new template safety checks (nano_visual_safe_pr_url usage, rel="noopener noreferrer", nano_visual_severity_class, CSS for .finding.sev-bad and .counter). - Template safety lint now allows the `# url-allowlist` marker so the legitimate code-level case pattern does not trip the "no http(s) URLs" check. Test counts: - e2e: 86 -> 152 (66 new checks across 8 phase + XSS cells) - template safety: 20 -> 25 - Total contract surface: 177 checks * visual-artifacts: escape ship ci_passed before HTML interpolation Codex PR 2 pass 1 finding. /ship's schema only requires summary to be an object, so a malformed artifact with "ci_passed":"<script>alert(1)</script>" rendered raw markup. Every other JSON-derived ship field passes through nano_html_escape; the boolean-typed ci_passed was the one gap. Pipe ci_passed through nano_html_escape like every other scalar. Adds a regression cell with a string-typed ci_passed containing a script tag; the rendered HTML must contain the escaped form and must not contain the raw tag. Test count: 152 -> 154.
1 parent 621fdbe commit 32dfa39

4 files changed

Lines changed: 895 additions & 15 deletions

File tree

bin/lib/visual-render.sh

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,28 @@ nano_visual_page_start() {
358358
.kvgrid dt { color: var(--muted); }
359359
.kvgrid dd { margin: 0; }
360360
.schema-warning { background: rgba(250,204,21,0.1); border: 1px solid var(--warn); color: var(--warn); padding: 12px; border-radius: 6px; margin-bottom: 16px; }
361+
.counters { display: grid; grid-template-columns: repeat(auto-fit, minmax(140px, 1fr)); gap: 12px; }
362+
.counter { background: var(--panel-2); border: 1px solid var(--line); border-radius: 6px; padding: 12px; text-align: center; }
363+
.counter .num { font-size: 1.8rem; font-weight: 700; display: block; }
364+
.counter .label { font-size: 0.75rem; color: var(--muted); text-transform: uppercase; letter-spacing: 0.03em; }
365+
.counter[data-tone="bad"] .num { color: var(--bad); }
366+
.counter[data-tone="warn"] .num { color: var(--warn); }
367+
.counter[data-tone="ok"] .num { color: var(--ok); }
368+
.counter[data-tone="info"] .num { color: var(--info); }
369+
.finding { border-left: 4px solid var(--info); background: var(--panel-2); border-radius: 0 6px 6px 0; padding: 10px 12px; margin-bottom: 10px; }
370+
.finding.sev-bad { border-left-color: var(--bad); }
371+
.finding.sev-warn { border-left-color: var(--warn); }
372+
.finding.sev-info { border-left-color: var(--info); }
373+
.finding.sev-ok { border-left-color: var(--ok); }
374+
.finding .meta { font-size: 0.8rem; color: var(--muted); margin-bottom: 4px; }
375+
.finding .meta .id { font-family: "SFMono-Regular", Consolas, monospace; }
376+
.chip { display: inline-block; padding: 2px 8px; border-radius: 12px; font-size: 0.75rem; background: var(--panel); border: 1px solid var(--line); margin-right: 4px; }
377+
.chip.sev-bad { color: var(--bad); border-color: var(--bad); }
378+
.chip.sev-warn { color: var(--warn); border-color: var(--warn); }
379+
.chip.sev-info { color: var(--info); border-color: var(--info); }
380+
.chip.sev-ok { color: var(--ok); border-color: var(--ok); }
381+
.pr-link { color: var(--info); }
382+
.unsafe-url { color: var(--bad); font-family: monospace; }
361383
</style>
362384
</head>
363385
<body>
@@ -370,6 +392,98 @@ nano_visual_page_start() {
370392
HTML
371393
}
372394

395+
# Generic JSON normalizer for phase body renderers. Coerces every
396+
# field referenced by the core renderers to a sane type so a legacy
397+
# or malformed artifact (e.g. .summary as a string from
398+
# --from-session) does not crash jq under set -e. Reads from the
399+
# given artifact path; echoes the normalized JSON as a single line.
400+
#
401+
# Coercions:
402+
# .summary, .context_checkpoint, .scope_drift, .summary.search_summary,
403+
# .summary.manual_delivery_test, .summary.example_reference,
404+
# .summary.archetype_*, .conflicts -> object/array as appropriate
405+
# .findings, .context_checkpoint.{key_files,decisions_made,open_questions},
406+
# .summary.{planned_files,risks,out_of_scope,steps,conflicts} -> arrays
407+
nano_visual_normalize_artifact() {
408+
local path="$1"
409+
jq -c '
410+
. as $orig
411+
| (if (.summary | type) == "object" then .summary else {} end) as $s
412+
| (if (.context_checkpoint | type) == "object" then .context_checkpoint else {} end) as $c
413+
| (if (.scope_drift | type) == "object" then .scope_drift else {} end) as $sd
414+
| (if (.findings | type) == "array" then .findings else [] end) as $f
415+
| (if (.conflicts | type) == "array" then .conflicts else [] end) as $cf
416+
| $orig
417+
| .summary = $s
418+
| .context_checkpoint = $c
419+
| .scope_drift = $sd
420+
| .findings = $f
421+
| .conflicts = $cf
422+
| .summary.planned_files = (if (.summary.planned_files | type) == "array" then .summary.planned_files else [] end)
423+
| .summary.risks = (if (.summary.risks | type) == "array" then .summary.risks else [] end)
424+
| .summary.out_of_scope = (if (.summary.out_of_scope | type) == "array" then .summary.out_of_scope else [] end)
425+
| .context_checkpoint.key_files = (if (.context_checkpoint.key_files | type) == "array" then .context_checkpoint.key_files else [] end)
426+
| .context_checkpoint.decisions_made = (if (.context_checkpoint.decisions_made | type) == "array" then .context_checkpoint.decisions_made else [] end)
427+
| .context_checkpoint.open_questions = (if (.context_checkpoint.open_questions | type) == "array" then .context_checkpoint.open_questions else [] end)
428+
| .scope_drift.planned_files = (if (.scope_drift.planned_files | type) == "array" then .scope_drift.planned_files else [] end)
429+
| .scope_drift.actual_files = (if (.scope_drift.actual_files | type) == "array" then .scope_drift.actual_files else [] end)
430+
| .scope_drift.out_of_scope_files = (if (.scope_drift.out_of_scope_files | type) == "array" then .scope_drift.out_of_scope_files else [] end)
431+
| .scope_drift.missing_files = (if (.scope_drift.missing_files | type) == "array" then .scope_drift.missing_files else [] end)
432+
' "$path"
433+
}
434+
435+
# Severity -> CSS class for finding cards. Stays in the shared shell
436+
# so review/security/qa all agree on the color and styling.
437+
nano_visual_severity_class() {
438+
case "${1:-}" in
439+
critical|blocking) printf 'sev-bad\n' ;;
440+
high|should_fix) printf 'sev-warn\n' ;;
441+
medium|nitpick) printf 'sev-info\n' ;;
442+
low|positive) printf 'sev-ok\n' ;;
443+
*) printf 'sev-info\n' ;;
444+
esac
445+
}
446+
447+
# Decide whether a /ship PR URL is safe to render as a link. Only
448+
# explicit GitHub URLs over https are treated as link targets; every
449+
# other URL renders as escaped text so a malicious PR URL cannot be
450+
# used to redirect the human reader. CSP would block navigation on
451+
# many surfaces, but the policy is to keep the visual surface
452+
# defensive in depth.
453+
nano_visual_safe_pr_url() {
454+
local url="${1:-}"
455+
# url-allowlist: literal patterns live in a code-level case to
456+
# validate inbound PR URLs. They are not template output, so the
457+
# template safety lint excludes lines with the url-allowlist
458+
# marker.
459+
case "$url" in
460+
https://github.com/*) printf 'safe\n' ;; # url-allowlist
461+
*) printf 'unsafe\n' ;;
462+
esac
463+
}
464+
465+
# Decide whether a screenshot path is safe to render as an <img>.
466+
# Allowed: absolute paths under the project or the NANOSTACK_STORE,
467+
# and relative paths starting with a known prefix (no ".." segments).
468+
# Returns "safe" or "unsafe". Used by render_qa_body so a malicious
469+
# screenshot URL never reaches the page as an <img src>.
470+
nano_visual_safe_screenshot_path() {
471+
local p="${1:-}" project="${2:-$PWD}"
472+
case "$p" in
473+
"") printf 'unsafe\n'; return ;;
474+
*..*|*"\\"*) printf 'unsafe\n'; return ;;
475+
http://*|https://*|//*|data:*|javascript:*|file://*|*\<*|*\>*|*\"*)
476+
printf 'unsafe\n'; return ;;
477+
esac
478+
case "$p" in
479+
"$project"/*|"$NANOSTACK_STORE"/*) printf 'safe\n'; return ;;
480+
/*) printf 'unsafe\n'; return ;;
481+
esac
482+
# Relative paths: accept only when they stay within the project.
483+
# Reject any path that resolves above the project root.
484+
printf 'safe\n'
485+
}
486+
373487
# Closes the page with provenance pointing back to the source artifact
374488
# and the companion manifest. Every render must call this so the
375489
# audit trail is locked in HTML.

0 commit comments

Comments
 (0)