fix(review): close post-merge security and evidence gaps

[monkey1sai]

Summary

• Follow-up to fix(deploy): harden transactional test rebuild #359, which was merged at head 8db52e7 before two late review fixes were pushed. This PR contains only those post-merge fixes on top of current origin/main@b9c88bf.
• Reject option-shaped origins and raw ? / # markers before URI, SCP-like, rooted-path, or relative-path classification. This closes relative-origin credential bypasses such as repo.git?access_token=... while preserving safe relative mirrors such as .\repo-mirror\AI-BIM-governance.git.
• Run functional producer -> Hi-Fi runtime slice -> commit-bound validator in that order. Hi-Fi actual/diff images now go to ignored artifacts/e2e/_output/; tracked golden PNGs are read-only baselines and every capture enforces a <=1% pixel-diff gate.
• No docs/plans content is changed. Current docs/plans remains the sole product authority. No A4 worktree/branch code and no Claude Code branch/worktree code was read into, cherry-picked into, or modified by this PR.

Scope / conflict review

Item	Result
Base	origin/main@b9c88bf, including merged #359 and #363
Commits	274c56f, e21e22f
Net changed files	5
A4 branch/worktree	excluded
Claude Code branch/worktree	excluded and untouched
Docs authority	docs/plans/*; no docs delta in this follow-up

AI Coding Governance

Item	Result
Change lane	S
Behavior contract changed	yes
Linked issue	post-merge follow-up to PR #359 review threads
Requirement source	docs/plans product authority plus existing deploy-security and functional-evidence contracts
CODEOWNERS / owner review	requested
GitNexus evidence	detect_changes(scope=compare, base_ref=origin/main) = LOW; 5 files, 0 affected processes
Browser E2E evidence	Playwright: functional producer 1/1 plus Hi-Fi runtime 4/4
Agent workflow changed?	yes — Hi-Fi now runs before commit-bound validation; rollback is revert this PR
Required checks expected	CI / Agent Governance / PR Review Agent / PowerShell static analysis / secret pattern scan

Frontend Verification

Expected machine scope is gate_infrastructure_only: this PR changes a Playwright evidence producer, not product UI source. It does not claim frontend or design-system completion.

Item	Result
Frontend route	/ui, #conv, #/demo-control, #/kit — regression evidence only
Main button(s) tested	conv-refresh
Fixture used	test-owned real coordinator plus deterministic external conversion/IFC stubs
Backend API called	POST /api/external/ifc-ready, GET /api/external/ifc-ready
Runtime action	load conversion result; observed runtime ID stream_conv_fixture_001
Visible success state	loading, success, failure, and retry all observed
E2E command	npx playwright test --config=playwright.functional-runtime.config.ts; npx playwright test e2e/hifi-token-authority.spec.ts --config=playwright.config.ts
Screenshot / trace	artifacts/e2e/functional-runtime/conv-history.png, conv-history-trace.zip, and artifacts/e2e/_output/hifi-token-authority/*-{actual,diff}.png
Design gate status	not required for gate_infrastructure_only; no product visual source changed
Design screen(s)	none added or modified
Reference-missing route(s) / surface(s)	none introduced
Full completion claimed	no
Design reference manifest	docs/plans/design-system-reference.manifest.json remains unchanged
Visual fidelity result	Hi-Fi supplemental ratios: 0 to 0.00135091; this does not replace the formal manifest gate
Visual comparison	Chromium DPR1; tracked baseline vs current actual; pixel diff <=1%
Visual artifacts	ignored _output actual/diff pairs; tracked baseline bytes remain unchanged
Known gaps	no full frontend, Docker, Kit/WebRTC, lineage, or full-system completion claim

Deploy Path Verification

Item	Result
Affects runtime / docker / Kit / viewer / ports / env?	deploy tooling origin validation and CI evidence only; no compose, service API, port, or env-value change
Canonical deploy path updated?	scripts/lib/rebuild-test-deploy.ps1 validation hardened; canonical wrapper/deploy path unchanged
New root script added?	no
Deploy dry-run command	exercised by the rebuild transaction suite
Full deploy tested	not run; fixed-path fresh-main rebuild cannot validate this unmerged follow-up
Verify command	PS7 and Windows PowerShell 5.1 scripts/tests/test-rebuild-test-deploy.ps1
Frontend URL verified	not applicable to a product completion claim
Evidence path	local test output plus PR CI artifacts

Validation

• pwsh -NoProfile -NonInteractive -ExecutionPolicy Bypass -File scripts/tests/test-rebuild-test-deploy.ps1 — PASS.
• powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -File scripts/tests/test-rebuild-test-deploy.ps1 — PASS.
• pwsh -NoProfile -NonInteractive -File scripts/tests/test-functional-runtime-result.ps1 — RED before the golden-protection implementation, then PASS.
• pwsh -NoProfile -NonInteractive -File scripts/tests/test-agent-governance-check.ps1 — PASS.
• npm run typecheck — PASS.
• targeted ESLint for e2e/hifi-token-authority.spec.ts — PASS.
• npm run build:ui — PASS.
• functional producer — PASS, 1/1; subject commit e21e22f06569fd081017691caa5525d075c7cb51.
• Hi-Fi runtime slice — PASS, 4/4.
• Hi-Fi pixel ratios: unified-home=0; remaining four screens 0.00130425 to 0.00135091, all below 0.01.
• verify-functional-runtime-result.ps1 -TargetCommit e21e22f06569fd081017691caa5525d075c7cb51 -AllowUntrackedArtifacts — PASS after producer and Hi-Fi; tracked working tree remained clean.
• git diff --check origin/main...HEAD — PASS.
• GitNexus compare — LOW, 5 files, 0 affected processes.
• Read-only Terra reviewer — APPROVE; read-only Sol reviewer — APPROVE; coordinator final adjudication — APPROVE. No unresolved BLOCKER/HIGH/MEDIUM finding in this follow-up.

Known Risks

• PSScriptAnalyzer is not installed locally, so no local pass is claimed; the Windows CI job installs it and remains authoritative.
• Full fixed-path deployment and Docker/Kit/WebRTC runtime were not rerun because this follow-up changes validation and evidence tooling, not those service runtimes.
• The inherited pre-cutover partial-service-stop recovery limitation from fix(deploy): harden transactional test rebuild #359 remains a documented residual MEDIUM: safe automatic restart still requires authoritative executable/args/env data and is not invented here.
• This PR does not implement the remaining rvt-ifc-usdc-lineage runtime or canonical real-MinIO #/pipeline browser E2E.

[coderabbitai]

Warning

Review limit reached

@monkey1sai, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ebf35441-c3fa-457a-af1b-72e3f99af4e7

📥 Commits

Reviewing files that changed from the base of the PR and between b9c88bf and e21e22f.

📒 Files selected for processing (5)

• .github/workflows/ci.yml
• scripts/lib/rebuild-test-deploy.ps1
• scripts/tests/test-functional-runtime-result.ps1
• scripts/tests/test-rebuild-test-deploy.ps1
• web-viewer-sample/e2e/hifi-token-authority.spec.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/non-a4-late-review-fixes

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[monkey1sai]

Final adversarial review

Verdict: no BLOCKER, HIGH, or MEDIUM finding in this follow-up PR scope.

• Requirements/scope: the diff is limited to the two post-merge fixes from #359 and remains based on current main@b9c88bf; no docs/plans, A4, or Claude Code branch/worktree content is included.
• Security/correctness: origin validation rejects option-shaped values and raw query/fragment markers before relative/rooted/SCP/URI classification; safe relative mirrors remain covered.
• Evidence integrity: tracked Hi-Fi PNGs are read-only baselines, current actual/diff files go to ignored _output, dimensions are checked before pixelmatch, and all five captures enforce diffPixelRatio <= 0.01.
• Commit binding: CI order is functional producer -> Hi-Fi -> target-commit validator. Local replay against e21e22f06569fd081017691caa5525d075c7cb51 passed and left the tracked tree clean.
• Independent review: Terra xhigh APPROVE; Sol max APPROVE; coordinator final adjudication APPROVE.

Evidence reviewed: PS7 and PS5.1 rebuild suites PASS; functional contract PASS; governance PASS; typecheck and targeted lint PASS; Playwright functional 1/1 and Hi-Fi 4/4 PASS; GitNexus compare LOW with 0 affected processes; local PR preflight PASS; GitHub CI and Agent Governance SUCCESS.

Residuals: CodeRabbit did not run because its external review quota was rate-limited, not because of a code finding. The inherited partial-service-stop recovery limitation remains documented and is not expanded by this PR.

[Copilot]

Pull request overview

This PR is a targeted post-merge follow-up to #359, applying two late review fixes on top of origin/main: a credential-leak hardening in the test-deploy origin validator, and an integrity upgrade to the Hi-Fi E2E visual-evidence flow. It does not touch product UI or docs/plans authority.

Changes:

• Hardens Assert-TestDeployOriginUrlSafe to reject option-shaped (-…) origins and any ?/# before URI/SCP/rooted/relative classification, closing the relative-origin credential bypass (e.g. repo.git?access_token=…) while still allowing safe relative mirrors (.\repo-mirror\…git).
• Reworks the Hi-Fi spec to route actual/diff screenshots into gitignored artifacts/e2e/_output/, treat tracked golden PNGs as read-only baselines, and enforce a ≤1% pixelmatch diff gate via a shared captureHifiVisualEvidence helper.
• Reorders CI so the Hi-Fi runtime slice runs before the commit-bound binding validator, and updates the governing test (test-functional-runtime-result.ps1) plus origin-validation tests to lock the new ordering and evidence contract.

Verification performed during review:

• pixelmatch (7.1.0) and pngjs (7.0.0) are declared dependencies; the old lowercase artifactDir binding is fully removed (new names use baselineArtifactDir/outputArtifactDir), and exactly one page.screenshot( call remains, satisfying the new assertion.
• CI step order (producer → hifi → validator) matches the IndexOf assertions; artifacts/e2e/_output/ is gitignored (artifacts/e2e/.gitignore:4) and the five tracked baselines exist and are force-tracked.
• The origin-validator refactor preserves prior reject behavior (the removed inline ?/# checks are now covered by the earlier global check) with no regression for safe origins.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
web-viewer-sample/e2e/hifi-token-authority.spec.ts	Adds captureHifiVisualEvidence helper: writes actual/diff to _output, reads tracked baselines, enforces ≤1% pixel-diff gate
scripts/lib/rebuild-test-deploy.ps1	Moves --prefix and ?/# origin rejection to the front of Assert-TestDeployOriginUrlSafe, closing the relative-origin credential bypass
scripts/tests/test-rebuild-test-deploy.ps1	Adds safe relative-mirror case and unsafe relative query/fragment credential-bypass cases
scripts/tests/test-functional-runtime-result.ps1	Locks new producer→hifi→validator ordering and asserts the _output/pixelmatch/single-screenshot evidence contract
.github/workflows/ci.yml	Reorders the Hi-Fi runtime slice to run before the binding validator

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.