Skip to content

Integrate reviewed human-review improvements - #1

Merged
erdemtuna merged 10 commits into
mainfrom
integration/reviewed-upstream-prs
Sep 2, 2026
Merged

Integrate reviewed human-review improvements#1
erdemtuna merged 10 commits into
mainfrom
integration/reviewed-upstream-prs

Conversation

@erdemtuna

Copy link
Copy Markdown
Owner

Summary

Validation

  • 120 unit tests passed with one Windows privilege skip
  • 9 Chromium browser tests passed
  • Windows restart path passed 20 consecutive runs
  • package contents and upstream ancestry verified

chinkeikk and others added 10 commits August 17, 2026 14:33
Apply a per-response nonce-based Content Security Policy so only the trusted Human Review SDK executes in local HTML and Markdown reviews. Authored scripts and inline handlers remain unchanged on disk but stay inert in the review surface. Add regression coverage for nonce injection, source round-tripping, and hostile in-frame scripts.
Add explicit comment editing, order comments by latest activity, update stranded feedback batches in place, and represent post-delivery revisions as corrections that survive acknowledgment. Include regression coverage for ordering, pending revisions, and correction delivery.
Use state.page.filename (already computed by pageState()) to set
document.title so browser tabs show which document is open instead
of all showing 'human review'.

Closes petergyang#24
The review iframe renders artifacts that are untrusted by definition, but
its sandbox granted allow-popups and allow-downloads. Neither is needed:
the SDK posts external links to the chrome over eh:external and the chrome
opens them, and nothing downloads. Removing both narrows what a malicious
artifact can do to the reviewer.

The eh:external handler also called window.open on an unvalidated href.
The origin check is the only thing in front of it, so a compromised frame
or a misconfigured policy could land a javascript: URL there, executing on
the chrome's origin alongside the session token. Route it through the
existing normalizeHref, which already allowlists http/https/mailto/tel and
is tested against javascript:, data:, vbscript: and tab-obfuscated
variants, rather than adding a second URL-safety helper to diverge from.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a956be5a-b08a-4a18-8034-1911ba9f56ac
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a956be5a-b08a-4a18-8034-1911ba9f56ac
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a956be5a-b08a-4a18-8034-1911ba9f56ac
Resolve the import overlap with PR petergyang#32 while preserving both features.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a956be5a-b08a-4a18-8034-1911ba9f56ac
Add exact batch receipts, transactional state updates, single-writer locking, restart-safe corrections, and deterministic shutdown without forced test exit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a956be5a-b08a-4a18-8034-1911ba9f56ac
Add nonce-hidden render capabilities, stale-frame rejection, split sandbox policy, redirect and title coverage, Playwright exploit tests, and bounded CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a956be5a-b08a-4a18-8034-1911ba9f56ac
@erdemtuna
erdemtuna merged commit 5d534f8 into main Sep 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants