Repository navigation
fix(web): never send a password typed inside an iframe to the authoring model - #797
Merged
Merged
Conversation
…cene Fails on purpose: the same-origin iframe pass reports el.value for every field, password included. The fix follows in the next commit.
HappyDevs1
added
the
full-ci
HappyDevs1
added
the
full-ci
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
The page-level scene already skipped password values because the scene travels to the authoring model; the same-origin iframe pass read el.value for every field. framed_scene_e2e (previous commit) now passes.
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
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.
The bug
While recording with an AI provider configured, the engine sends a "scene" (the page's fields and labels, with their current values) to the authoring model. The page-level pass has always left password values out (
fieldValue()skipstype="password"). The same-origin iframe pass did not: it reportedel.valuefor every field. So a password typed into a login form inside an iframe was sent to the AI provider, cut to 80 characters.Found while building the "what leaves your machine" inventory for the security page (automators-com/flowproof-desktop#306).
How it was verified
Reproduced locally by running
SCENE_JS, taken verbatim fromweb.rs, in headless Chrome against a page with a password on the page and a login form in a titledsrcdociframe, then typing a secret into both. The page password was absent from the scene, but the framed entry carried"text": "framed-secret-222".Fixed: the iframe pass now skips the value of
input[type=password], the same rule as the page. The field stays in the scene with its target and label, so recording can still find and fill it.Re-tested locally: both secrets are absent from the scene, and the framed non-password field still reports its value (
alice).CI, on purpose in two commits:
7e1300eaddsframed_scene_e2e.rsand wires it into the web E2E job. It should fail against the unfixed code.The
full-cilabel is set so the web E2E job runs.Also noticed (not changed here)
grid_scene_e2e.rs(which already asserts a page-level password stays out of the scene) andpassword_recording_e2e.rsare never run in CI. They return early unlessFLOWPROOF_E2E=1, and no job lists them. Worth wiring up separately.