Commit 621fdbe
authored
visual-artifacts v1: PR 1 static renderer and trust contract (#217)
* visual-artifacts: PR 1 static renderer and trust contract
Introduce bin/render-artifact.sh and the supporting libraries that
turn a Nanostack JSON artifact into a local, static HTML view under
$NANOSTACK_STORE/visual/. JSON remains canonical; the renderer is
strictly downstream and writes only to the visual root.
PR 1 wires the /plan renderer end to end. think, review, security,
qa, ship are reserved for PR 2 and exit 1 with a clear message;
journal and stack are reserved for PR 3 and exit 2; --interactive is
reserved for PR 4 and exits 2.
Key files:
- reference/visual-artifact-contract.md: normative contract.
- bin/lib/html-escape.sh: nano_html_escape, nano_attr_escape,
nano_json_string. Every JSON-derived scalar passes through one of
these before reaching HTML.
- bin/lib/visual-render.sh: shared page shell, CSP, locked trust
badge wording (verified / unverified / tampered), output path
safety, symlinked visual root rejection.
- bin/render-artifact.sh: argument parsing, source resolution via
bin/find-artifact.sh, trust verification via
bin/lib/artifact-trust.sh, schema validation via
bin/lib/artifact-schemas.sh, manifest writer, /plan body renderer.
Atomic write with $path.tmp.$$ rename.
- ci/e2e-visual-artifacts.sh: 49 checks across 9 cells (happy path,
XSS, --strict integrity_missing, integrity_mismatch always fails,
--out path safety, reserved features, --manifest-only, phase
mismatch, symlinked visual root).
- ci/check-visual-artifact-templates.sh: 20 static checks for
forbidden patterns (http://, https://, fetch, XMLHttpRequest,
localStorage, document.cookie, eval) plus required markers (CSP,
data-nanostack-visual, locked badge wording).
- .github/workflows/lint.yml: new visual-artifact-contract job that
runs both checks on every push.
Trust contract:
- integrity_mismatch always fails with exit 3.
- integrity_missing fails under --strict (exit 3); without --strict
the render proceeds and the badge shows 'unverified'.
- --out outside the visual root fails with exit 4.
- A symlinked visual root fails with exit 4.
Total: 69 contract checks locked in CI.
* visual-artifacts: fix legacy plan render and fresh-store --out
Two PR 1 pass 1 codex findings.
1. Legacy --from-session plan artifacts store .summary as a string,
so jq -r '.summary.goal' aborted the body renderer under set -e.
The schema warning was already emitted in the page head, but the
body never reached the user. Normalize .summary and
.context_checkpoint into objects at the top of render_plan_body
(defaulting missing arrays to empty arrays), then read every
field from the normalized JSON. Adds two regression cells:
--from-session legacy plan + an artifact with both summary and
context_checkpoint as strings.
2. --out under $NANOSTACK_STORE/visual was rejected on fresh stores
because the visual root did not yet exist; the canonical
walk-up landed on $NANOSTACK_STORE, which is outside the visual
root. Pre-create the visual root before the safety check so
realpath has a stable target. Adds a regression cell that
confirms --out works when visual/ does not pre-exist.
Test count: 49 -> 59 (cells 9a, 9b cover the regressions).
* visual-artifacts: reject --out paths that escape visual/ via ".."
Codex PR 1 pass 2 caught a path-safety gap. A path like
$NANOSTACK_STORE/visual/new/../../outside.html passed the previous
"walk up to nearest existing ancestor" check because the 'new'
segment was missing on disk; the walk landed on visual/, the prefix
matched, and the final mv wrote to $NANOSTACK_STORE/outside.html
outside the visual root.
Replace the realpath walk-up with lexical normalization. The new
nano_visual_normalize_path resolves "." and ".." string-wise so
non-existent intermediate segments do not anchor the comparison.
Both the candidate path and the visual root are normalized
lexically; symlink protection on the visual root itself is still
provided by nano_visual_assert_safe_root.
Adds a regression cell that exercises the documented escape and a
check that no file is left behind outside visual/ after the
rejection.
Test count: 59 -> 61.
* visual-artifacts: canonicalize manifest paths under relative store
Codex PR 1 pass 3 finding. A relative NANOSTACK_STORE override
(e.g. NANOSTACK_STORE=.nano-rel) propagated into the renderer's
output_path, source path, and stdout. The visual artifact contract
requires output_path to be absolute, so the manifest emitted under
a relative store violated the contract.
After mkdir -p of the parent directories (so $(cd .. && pwd) has a
real target), resolve HTML_PATH, MANIFEST_PATH, and ART_PATH to
absolute via a small nano_resolve_abs helper. The fix runs late
enough that the path-safety check still operates on the caller's
literal --out string, and the manifest writer sees only absolutes.
Adds cell 9d: chdir to a fresh project, set NANOSTACK_STORE to a
relative ".nano-rel", render, and assert the stdout, manifest
output_path, and manifest source path are all absolute.
Test count: 61 -> 64.
* visual-artifacts: reject symlinked subdirectories under visual/
Codex PR 1 pass 4 finding. A pre-existing symlink under visual/
(for example visual/plan -> /tmp/outside) was accepted by mkdir -p
and the renderer's atomic mv then wrote the HTML or manifest into
the symlink target. nano_visual_assert_safe_root only guarded the
root itself, so the new path-safety contract was incomplete.
Add nano_visual_assert_safe_descend: walk from the visual root down
to (but not including) the leaf file, asserting -L is false at every
intermediate. render-artifact.sh calls it for both the HTML path and
the manifest path before mkdir -p runs.
Adds two regression cells: a symlinked visual/plan and a symlinked
visual/manifests, each confirming exit 4 and no file written
through the symlink target.
Test count: 64 -> 68.
* visual-artifacts: reject symlinked or directory leaves at output
Codex PR 1 pass 5 finding. The descend check walked every
directory component but stopped before the leaf, so an --out whose
final segment was a pre-existing symlink to a directory escaped
the path-safety contract: the atomic mv moved the temp file INTO
the symlink target instead of overwriting the link.
Extend nano_visual_assert_safe_descend to refuse leaves that are
symlinks (return 4) or directories (return 4). The renderer writes
a regular file at that path; symlinks and directories at the leaf
were never part of the contract.
Adds cell 9g: a symlinked leaf at --out and a directory at the
--out path. Both must exit 4 and leave no file in the symlink
target.
Test count: 68 -> 71.
* visual-artifacts: write to normalized path and lock other contracts
Three findings from codex PR 1 pass 6.
P1 — write to the normalized path
A path like --out $NANOSTACK_STORE/visual/link/../evil.html with
link a symlink to /tmp/outside collapsed lexically to visual/
evil.html and passed the safety check. The kernel still resolved
the original path at write time: it followed link to /tmp/outside,
took .. back to /tmp/, and wrote evil.html outside the visual root.
Reassign HTML_PATH and MANIFEST_PATH to their normalized form
before mkdir -p / mv so the kernel never traverses a `..` after a
symlinked component.
P2 — unique manifest stem per render
Two same-second renders shared a manifest stem, so the second
render overwrote the first manifest while the first HTML kept
pointing at the now-stale path. Append the renderer's PID to the
timestamp stem; each render-artifact.sh invocation is its own
process so the stem is unique across same-second invocations.
P3 — non-object JSON exits 1
A top-level array, string, or number artifact crashed jq -r
'.phase // ""' with exit 5 under set -e, violating the CLI
contract (exit 1 for input errors). Switch to `.phase?` so the
path error is suppressed and the existing phase-mismatch branch
returns exit 1 cleanly.
Adds three regression cells covering the symlink+.. bypass, the
same-second manifest uniqueness, and the three non-object JSON
shapes (array, string, number).
Test count: 71 -> 79.
* visual-artifacts: render does not mutate sprint session state
Codex PR 1 pass 7 finding. find-artifact.sh registers the producing
phase via session.sh phase-start as a convenience for downstream
skills; render-artifact.sh hitting that code path through --latest
made a strictly-downstream viewer mutate session.json. A user who
opened the latest plan as HTML would silently start a plan phase.
Add --no-session-sync to find-artifact.sh. The flag bypasses the
phase-start side effect while preserving every other behavior
(integrity checks, max-age filter, project matching).
render-artifact.sh now uses --no-session-sync on its --latest
lookup, restoring the read-only contract documented in
reference/visual-artifact-contract.md.
Adds cell 9l: start a session, snapshot phase_log, run a render,
assert phase_log is unchanged and plan is not flagged in_progress.
Test count: 79 -> 81.
* visual-artifacts: secure temp files and glob-safe path normalization
Codex PR 1 pass 8. Two findings.
P2 — predictable temp file
Temp filenames followed a guessable pattern ($HTML_PATH.tmp.<pid>).
An attacker with write access to a parent directory could
pre-create a symlink at that path, and bash's > redirect would
follow the symlink and write outside visual/.
Replace the manual temp naming with mktemp("$path.tmp.XXXXXX").
mktemp opens with O_EXCL so a pre-existing symlink races into a
clear failure (exit 4) instead of silent follow. The cleanup trap
keeps unlinking on early exit. Cell 9n verifies no .tmp.* leftover
after a successful render.
P3 — glob expansion during normalization
The IFS split inside nano_visual_normalize_path and
nano_visual_assert_safe_descend used an unquoted `set -- $raw`,
which performs pathname expansion against the current working
directory. An --out like "star*.html" could be silently rewritten
to a matching real filename, so the renderer wrote to a different
path than the caller asked for and the manifest recorded the wrong
output_path.
Save the current `set -f` state, disable globbing for the split,
and restore the previous state when the helpers return. Cell 9m
locks the contract: a literal glob path stays literal.
Test count: 81 -> 85.
* visual-artifacts: clean HTML temp under --manifest-only
Codex PR 1 pass 9. After the PR 1 pass 8 switch to mktemp for the
temp files, the --manifest-only branch moved the manifest into
place and disabled the cleanup trap, leaving the HTML temp file
behind. The intent of --manifest-only is "write no HTML artifacts",
so the leftover violated the contract and would accumulate stale
*.html.tmp.* files under visual/plan/ on every CI trust-check run.
Remove TMP_HTML explicitly in the --manifest-only branch before
disabling the trap. Cell 7 now also asserts zero *.tmp.* files
remain after a --manifest-only render.
Test count: 85 -> 86.1 parent 90ca3d3 commit 621fdbe
8 files changed
Lines changed: 1931 additions & 2 deletions
File tree
- .github/workflows
- bin
- lib
- ci
- reference
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3766 | 3766 | | |
3767 | 3767 | | |
3768 | 3768 | | |
| 3769 | + | |
| 3770 | + | |
| 3771 | + | |
| 3772 | + | |
| 3773 | + | |
| 3774 | + | |
| 3775 | + | |
| 3776 | + | |
| 3777 | + | |
| 3778 | + | |
| 3779 | + | |
| 3780 | + | |
| 3781 | + | |
| 3782 | + | |
| 3783 | + | |
| 3784 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
18 | 27 | | |
19 | 28 | | |
20 | 29 | | |
| |||
32 | 41 | | |
33 | 42 | | |
34 | 43 | | |
| 44 | + | |
35 | 45 | | |
36 | 46 | | |
37 | 47 | | |
| |||
46 | 56 | | |
47 | 57 | | |
48 | 58 | | |
| 59 | + | |
49 | 60 | | |
50 | 61 | | |
51 | 62 | | |
| |||
73 | 84 | | |
74 | 85 | | |
75 | 86 | | |
76 | | - | |
| 87 | + | |
77 | 88 | | |
78 | 89 | | |
79 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
0 commit comments