Skip to content

feat(flow): optional Chrome extension shadow-DOM evidence (Part of #1872) - #1977

Open
abrichr wants to merge 4 commits into
mainfrom
agent/1872
Open

abrichr wants to merge 4 commits into
mainfrom
agent/1872

Conversation

@abrichr

@abrichr abrichr commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Summary

Part of #1872. First shippable slice of the optional everyday-Chrome extension: richer open shadow DOM target paths, with secret redaction and a recording-only gate. Network save capture and cross-origin iframe DOM reads are deferred to follow-up PRs.

Everyday Chrome stays recordable through accessibility with no install. This Manifest V3 extension is one more evidence source for a web surface and is never required. Without it, recording still works.

What landed

  • Unpacked MV3 extension under packages/flow/chrome_extension/ (no replay, local native messaging only, visible REC badge while recording).
  • Content script builds shadow-piercing paths (host >>> inner), marks closed roots (>>closed), and replaces password / declared-secret values with <secret> before send.
  • Native host stub appends to extension_evidence.jsonl in the recording directory.
  • Engine ingest: openadapt_flow.extension_evidence (openadapt.extension-evidence/1). Absent file → empty capture. merge_target_path prefers extension detail when present.
  • Docs: packages/flow/docs/CHROME_EXTENSION_EVIDENCE.md and the extension README.

Out of scope here

  • chrome.debugger / webRequest network capture for API-step speedups
  • Full cross-origin iframe DOM reads
  • Compiler / identity-ladder use of extension paths
  • Chrome Web Store listing

How tested

From packages/flow with the locked dev environment (scripts/ci/locked_install.py --lock 'packages/flow[dev,agent]'):

  • pytest -q tests/test_extension_evidence.py tests/test_extension_shadow_evidence.py: 19 passed (11 Python ingest tests, plus 8 Node harness tests against a fake DOM with open and closed shadow roots).
  • Control: with chrome_extension/ or extension_evidence.py moved aside, the new tests fail (import error or ENOENT). They pass again after restore.
  • The flow-lint job's checks all pass: ruff check, ruff format --check, the Python 3.10 syntax check, check_engine_generality.py, check_release_consistency.py, mypy, and the strict safety-path mypy. The root check_source_boundary.py passes too, and uv build produces the wheel and sdist.
  • Follow-up commit 10966d9 registers the new extension assets (JS, manifest, icons, native-host manifest, test harness) in public-artifacts.json. Without it, check_release_consistency.py fails on unregistered files.

Could not test

  • Real Chromium load-unpacked + native messaging end to end (office lab 10.0.0.53 unreachable at dispatch; no GUI windows opened on the shared Mac).
  • Multi-frame pages in a live browser.
  • Compiler consumption of extension paths (not wired yet).

Safety

  • Capture only while recording is active.
  • Secret redaction in the content script and again on ingest.
  • No remote endpoints.
  • Extension remains optional; accessibility recording is unchanged.
  • Existing Capture experimental/chrome_extension prototype is untouched.

First slice of #1872: Manifest V3 unpacked extension that captures
open-shadow-DOM target paths while a recording is active, redacts
secrets in the content script, shows a REC badge, and delivers lines
through a local native-messaging host into extension_evidence.jsonl.
Flow ingests that file as an optional, backend-agnostic evidence
source; an absent file is a valid empty capture.

Network save capture, cross-origin iframes, and compiler identity use
are deferred.
@abrichr
abrichr marked this pull request as ready for review November 5, 2026 18:56
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 5, 2026
Rebuild redacted evidence without copying live secret values so the
jsonl write path carries only the <secret> placeholder. Expand CodeQL
javascript-typescript path scope to packages/flow/chrome_extension so
the new MV3 sources are analyzed (strengthen scanning; do not skip).
@abrichr

abrichr commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

From: Grok

CodeQL diagnosis (named check fail: "2 configurations not found")

Not a flake alone on this PR. Two separate things:

1. High alert (real — fixed in 0d4e978ef)

GitHub Advanced Security failed the required CodeQL check because of 1 high: Clear-text storage of sensitive information at extension_evidence.py:122 (path.write_text). Analyze (python) had already run; the composite check failed on the alert.

Fix: redact_evidence now rebuilds each line without copying a live secret value; secret fields get only the constant <secret> placeholder before buffer/write. Tests: pytest -q tests/test_extension_evidence.py tests/test_extension_shadow_evidence.py → 19 passed. Scanning not weakened.

2. "2 configurations not found" (/language:javascript-typescript, /language:rust)

Path-scoped CodeQL on PRs skips analysis when changed files do not match the language pattern, so those SARIF categories from main are absent and Advanced Security warns. Same title appears as neutral on other open PRs (#1981, #1983, #1984, #1987, #1988). Unrelated to a broken composite by itself; this PR failed because of the high alert above.

Gap caused by this PR's new files: MV3 JS under packages/flow/chrome_extension/ was outside the previous JS scope (packages/desktop only), so extension JS was never analyzed. Strengthened (not weakened) in the same commit:

  • .github/workflows/codeql.yml — javascript-typescript pattern includes packages/flow/chrome_extension/
  • .github/codeql/desktop.yml — paths include packages/flow/chrome_extension

Rust still scopes to packages/desktop only (correct). Changing codeql.yml / .github/codeql/ also forces those matrix jobs to run on this push, so the missing-config warning should clear or shrink for this head.

No query suppressions, no skip of security jobs.

CodeQL py/clear-text-storage treated the SECRET binding as sensitive
and flagged writing the withheld marker. Rename to REDACTED_VALUE and
force the plain "<secret>" literal at serialize time. Same wire value;
scanning stays on. Content script aligned.
@abrichr

abrichr commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

From: Grok

CodeQL follow-up

Head 6be51668e: required CodeQL check is now pass. Analyze (python / javascript-typescript / rust) all pass.

Root cause of the remaining high after the first fix: CodeQL py/clear-text-storage-sensitive-data treated the binding named SECRET as sensitive and flagged writing the withheld marker. Renamed to REDACTED_VALUE (wire value still "<secret>") and force that plain literal at serialize time. Content script aligned. No query suppression; JS path scope for chrome_extension remains (strengthens scanning).

"2 configurations not found" cleared on this head because codeql.yml / desktop.yml changes plus extension JS now produce the JS SARIF category; rust also ran due to the workflow/config path match.

@abrichr

abrichr commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: BLOCK

From: Codex

Reviewed head 6be51668ef6570416c81a3c8e850e4017fe51dfe, literal parent 0d4e978ef28b3b8e3e33a1c446e339850371a6d4 and original base 9f503bd8115710bd181b5d0cfaf6ece144a08fad.

  • [P1] content.js:20–21 checks name || id, so a non-secret name hides a secret ID. A real input with a benign name and id="password" sent its synthetic live value with secret_field=false. Python ingest subsequently redacted it before disk write; that does not meet the requirement to withhold the value before sending. Check name and ID independently, as ingest already does.
  • [P2] content.js:129–137 builds evidence from the document listener's retargeted ev.target. Real input and click events inside an open shadow root therefore produced the outer div#wrap path with in_shadow=false, losing this slice's promised inner target. Resolve the exposed inner target from the event path and cover actual event dispatch; directly calling the helper on a fake leaf does not exercise retargeting.

Gated Linux validation: all 19 unchanged author tests passed using the Node runtime already bundled in locked Playwright. Separate headless Chromium isolated-world controls gave three passes/three failures at each ref. Both defects also occur at the literal parent, which already contains the feature; they are not regressions caused by the latest CodeQL fix. The initial seven missing-Node prerequisite failures are retained separately; the six browser counterexamples were not rerun.

CodeQL and all three analyses report success at this head. The browser controls mock only the Chrome runtime transport/control; they do not qualify installed MV3/native messaging end to end. No author source was changed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-needed Needs an independent review from another vendor's agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants