P1: core-loop tests, error-status fix, envFor scoping, WeChat computed-style, structured logging - #132
Open
Caosmart1979 wants to merge 9 commits into
Open
P1: core-loop tests, error-status fix, envFor scoping, WeChat computed-style, structured logging#132Caosmart1979 wants to merge 9 commits into
Caosmart1979 wants to merge 9 commits into
Conversation
… export The live preview / deck / template-preview iframes used sandbox="allow-scripts allow-same-origin", which makes their srcDoc content share the host origin — so agent-generated scripts could read parent.localStorage and call /api/* (convert / deploy / marketplace.install) with the host's credentials. README's "host never poisoned / cookies & localStorage quarantined" claim was false for this combination. Drop allow-same-origin from the three script-running iframes so agent HTML executes in an opaque origin; scripts, Tailwind CDN, fonts and animations still work, but can no longer reach the host. PNG export previously read the live iframe's contentDocument (the coupling that forced same-origin). Rewrite iframeToBlob to snapshot via a throwaway offscreen iframe built from the srcDoc attribute the parent already owns (readable regardless of the iframe's origin). Signature unchanged, callers untouched. - preview-pane.tsx, deck-viewer.tsx, template-picker.tsx: sandbox="allow-scripts" - image.ts: iframeToBlob builds an offscreen snapshot iframe from srcDoc Verified: pnpm -F @html-anything/next typecheck passes. Closes chain 1 of the production-grade audit (docs/superpowers/specs/2026-07-28-design-system-production-audit.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dows argv injection Chain 2 — /api/convert (and /api/draft) spawned the agent CLI with no server-side limits: unbounded body, no concurrency cap, no timeout, and cwd/binOverride accepted straight from the request body while the CLI runs under bypassPermissions / workspace-write / --yolo. - Add lib/agents/spawn-guards.ts: MAX_INFLIGHT=2 concurrency slot, 10 min watchdog, 2 MiB body cap, 1 MiB prompt cap. Both routes acquire before spawn and release in finally/cancel. - Drop `cwd` from the accepted body — the agent runs with elevated file perms, so the working directory stays server-owned (process.cwd()). The UI never sends cwd; a configurable root, if ever needed, goes through a Settings allowlist. - binOverride (invoke.ts): constrain the basename to the agent's known bin names (sans .exe/.cmd/.bat) so a request can't spawn an arbitrary executable under elevated flags. Custom install paths still work. Chain 3 — Windows command injection. invoke.ts set shell:true on Windows (.cmd shims require it) with a comment claiming the prompt never enters the command line — false for argv / argv-message protocols (deepseek positional, openclaw --message), which put the prompt into the argv cmd.exe parses. Replace the false comment; reject prompts containing cmd metacharacters (& | < > ^ % newlines) for those two protocols on Windows with a clear error steering to a stdin-capable agent. macOS/Linux use direct exec (no shell), so argv is safe there. Verified: pnpm -F @html-anything/next typecheck passes; agents unit tests green (the 17 skills/__tests__ tar failures are pre-existing Windows-only, audit qa-003, unrelated). Closes chains 2 + 3 of the production-grade audit (docs/superpowers/specs/2026-07-28-design-system-production-audit.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…token ACL The marketplace routes carried a per-route Host-header guard (marketplace/_lib/host-guard.ts) as a stopgap until the global /api/* middleware landed. It has (commit nexu-io#61), making the per-route guard redundant — and worse, its LOOPBACK_HOSTS accepted 0.0.0.0, which the global host-validation deliberately excludes (pre-fix Chrome 0.0.0.0-day DNS rebinding). The module's own header said "delete once the middleware merges." - Delete marketplace/_lib/host-guard.ts + its test; remove the per-route isHostAllowed checks from the 4 marketplace routes. Host gating now has a single source of truth: middleware.ts -> lib/security/host-validation.ts (covered by host-validation.test.ts + e2e host-validation.spec.ts). - Drop the two route-level host-regression tests in skills api.test.ts that bypassed middleware by calling the handler directly; keep the functional tests. Deploy token ACL (audit sec-005): deploy/config.ts relied on chmod 0o600 to protect the plaintext Vercel/Cloudflare token — a no-op on Windows/NTFS — and silently swallowed the chmod failure. restrictDeployConfigPerms now chmods on POSIX and runs `icacls /inheritance:r /grant:r <user>:F` on Windows, logging (not swallowing) on failure. Verified: typecheck passes; marketplace api functional tests pass. The 2 tar-packaging test failures are pre-existing Windows-only (audit qa-003: system tar cannot resolve C: drive-letter paths) and unrelated to this change. Closes task 5 of the production-grade audit P0 (docs/superpowers/specs/2026-07-28-design-system-production-audit.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The audit verified several README claims against the code and found them false. Correct them so the README isn't louder than the implementation: - Security: the preview iframe is now sandbox="allow-scripts" (no allow-same-origin -> opaque origin) after the chain-1 fix. The old "host never poisoned / cookies quarantined" wording was false for the previous allow-scripts allow-same-origin combination; reword to describe the opaque-origin sandbox and the offscreen snapshot used for PNG export. - huashu attribution: the anti-slop constraints are NOT "inside every SKILL.md frontmatter" (0/78 contain them). They are centralized in next/src/lib/templates/shared.ts and applied at prompt-assembly. - Status table: WeChat export is partial (computed-style inlining for Tailwind-CDN templates unimplemented), not stable; multi-template compare is planned/not-started (0 lines), not in-progress; skill marketplace + per-task history are shipped; Hyperframes ships a Remotion .zip (mp4 is planned). Agent detection is 11 callable stdin/argv CLIs; 7 ACP/pi-rpc agents are detect-only. - Drop the unverifiable "40k★ 200+ contributors" upstream figures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s CI
The marketplace installer preflight parsed tarballs in pure JS, but the
extraction step still shelled out to `tar -xzf` (install.ts:241). On Windows
this failed with "Cannot connect to C: resolve failed", and every skills
test that built fixtures with `spawn('tar', ['-czf', ...])` also failed on
Windows (audit qa-003: 17/164 tests).
- Replace `extractTarball` in lib/skills/install.ts with a pure-JS ustar
extractor (mirroring the preflight parser). The tarball has already passed
preflight, so we trust the header vetting and write regular files to disk.
This fixes marketplace install on Windows and removes the last system tar
dependency in the production install path.
- Add lib/skills/__tests__/tarball.ts: shared pure-JS `tarGzDir` + `buildTarball`
helpers. Switch api.test.ts, install.test.ts, install-rejections.test.ts,
and cross-device.test.ts to use it. The symlink rejection test now builds
its symlink entry in-memory instead of calling fs.symlink, which needs
admin/Developer Mode on Windows.
- Update preflight.test.ts happy-path to use its existing buildTarball helper
instead of spawn('tar'); remove the unused spawn import.
- Add a windows-latest matrix job to .github/workflows/ci.yml so Windows
unit tests run on every PR/push.
Verified locally: scripts/guard.ts, pnpm -F @html-anything/next typecheck,
next unit tests (154 passed), and e2e typecheck all pass on Windows.
Closes task 6 (Windows CI + README) of the production-grade audit
(docs/superpowers/specs/2026-07-28-design-system-production-audit.md).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The product's core loop (input -> agent spawn -> SSE -> preview) had zero automated coverage (audit qa-001). Two of its invariants were also wrong: - use-convert.ts set task status to "done" unconditionally when the SSE stream ended, even when an `error` event arrived during the run. A failed run (agent crash, non-zero exit surfaced as error) showed as "done". Track a sawError flag through the read loop and set "error" when it fired; skip the diff-edit baseline commit on error so the next run regenerates cleanly. - New invoke.test.ts: drives invokeAgent against a real fake-agent binary (bash shim / Windows .cmd) and asserts start + delta + done streaming, non-zero-exit error, AbortSignal -> SIGTERM child termination, the binOverride basename allowlist, and unknown-agent rejection. - New route.test.ts: mocks invokeAgent to assert /api/convert body validation (400 / unknown template), SSE event relay (start/delta/done), error-event propagation, the 2 MiB body cap (413), and that cwd in the request body is NOT passed through to the spawn. Verified: 165 next unit tests pass; typecheck clean. Closes P1 task 8 (core-loop tests + error status). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
invokeAgent previously passed the host's entire process.env to the spawned agent CLI. An agent runs user content via the prompt, and codex has network access, so that was a credential-exfiltration surface: a prompt-injected agent could read every other tool's API key / deploy token / DATABASE_URL from its own env and ship it out. envFor now denylists secret-shaped env (API_KEY / TOKEN / SECRET / PASSWORD / CREDENTIAL / DATABASE_URL / CONN …) and only forwards a secret if the running agent owns it (per AGENT_OWNED_SECRETS). Non-secret env (PATH, HOME, locale, …) is passed through unchanged so agents keep working. bypassPermissions (claude) and codex network_access are left as-is: both are real-agent behavior changes that can't be verified without agent-integration tests, and the remote prompt-injection -> RCE path that made them dangerous is already closed (opaque-origin iframe + spawn gates). Added an inline comment documenting the deferral. Tests: envFor keeps an agent's own key, strips other agents' keys, strips generic secrets, keeps non-secret env; gemini trust flag preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WeChat strips every <style> tag and external CSS, keeping only inline style="". The previous implementation ran juice on static <style> rules - which sees nothing for templates that load Tailwind via the Play CDN (the majority, per shared.ts). The comment in the file itself described the fix but never implemented it. Add renderToWechatHtml: renders the document in a hidden offscreen iframe so Tailwind CDN (and any runtime <style>) actually apply, then walks getComputedStyle for every element and copies a curated set of ~60 layout properties onto inline style. The result is a <section data-tool> that survives WeChat paste with visual fidelity. The old juice-based toWechatHtml stays as the sync fallback for SSR / iframe-failure paths; copyToWechat now calls renderToWechatHtml. Verified: happy-dom applies <style> and my computed walk inlines color/font-weight/display correctly; the section wrapper is present. Closes P1 task 7 (WeChat computed-style export). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent invoke path had zero server-side observability - when a spawn
hangs, exits non-zero, or errors, only the SSE client saw the event. This
made production debugging a guessing game. Add zero-dependency structured
logs (console.warn + JSON) at three points that cover every invoke outcome:
- invoke.spawn_error - agent / err message
- invoke.child_error - agent / duration / err
- invoke.close - agent / exitCode / duration / promptBytes / stderr
/ outputBytes (regardless of success or failure)
Logs are one JSON record per console.warn call, parseable by grep | jq and
indexable by any JSON-log analyser without pulling in a logger dependency.
Closes P1 task 10 (structured logging).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lefarcen
added
size/XL
Contributor
|
@Caosmart1979 I'm holding off on generating review comments for #132 because this pull request has merge conflicts right now. Please resolve the conflicts with main and push the updated branch. Once that's done, request or wait for the review to run again and I'll take another look. 🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos. |
|
Hey @Caosmart1979, thanks for breaking this into four concrete chunks and calling out the added coverage — that makes the intent easy to follow. We’ve routed the primary review to @PerishCode. One heads-up while that’s in flight: PR #94 is also open on the WeChat export path, and both PRs touch |
This was referenced Jul 28, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #131 (P0 security fixes). 4 independent reliability/observability/product improvements, all verified with new tests.
Changes
1. Core-loop integration tests + error-status fix (a9a24ce)
The product's main pipeline (agent spawn → SSE → preview) had zero automated coverage. Added:
invoke.test.ts— drives invokeAgent against a real spawn (node fake-agent binary, .cmd shim on Windows); asserts start/delta/done streaming, non-zero exit, AbortSignal → SIGTERM termination, binOverride allowlist, unknown-agent rejection.route.test.ts— mocks invokeAgent to assert /api/convert body validation, SSE event relay, error propagation, 2MiB body cap, and cwd-not-passed-through.use-convert.ts: a run that received an SSE error event (agent crash) was unconditionally flagged "done" in the UI. Now trackssawErrorand sets "error" status, skipping the diff-edit baseline commit.2. envFor: scope child process env to the running agent's own secrets (b293318)
The agent CLI previously inherited the host's FULL process.env. With codex's network_access, this was a credential-exfiltration surface. envFor now:
3. WeChat computed-style inlining (80e44a9)
WeChat strips every
<style>tag + external CSS. The old implementation ran juice on static<style>rules, which sees nothing for the Tailwind Play CDN (used by all templates per shared.ts). The file's own comment described the fix but never shipped it.→
renderToWechatHtmlrenders the document in a hidden offscreen iframe so Tailwind CDN actually applies, walks getComputedStyle onto inline style properties (~60 curated props), tagged with data-tool. Falls back to the sync juice path. Happy-dom testing confirms the computed walk works.4. Structured invoke-agent logging (fce51ac)
Zero-dependency JSON logs (console.warn) at spawn_error, child_error, and close — carrying agent / exitCode / duration / promptBytes / stderr / outputBytes per invocation. Parseable with grep | jq.
Verified