redact passwords only, and say what each step did in the log - #93
Draft
priyanshujain wants to merge 7 commits into
Draft
redact passwords only, and say what each step did in the log#93priyanshujain wants to merge 7 commits into
priyanshujain wants to merge 7 commits into
Conversation
The screen name was web-only: the Chrome driver stamps sanderling-screen on the root and nothing else does, so every Android and iOS step recorded and logged an empty screen. The route marker the tree already carries (the resource id ending in Screen, the same one Transitional counts) names it.
One line per step carried only an index and a node count. It now names the screen, the action, its target and the typed value, the last through the same redaction the trace and the prompt use. Emitted after the apply so the line reports what actually happened, skip reason included.
maestro's tree mapper copies a fixed attribute list off the device's XML and password is not on it, so no android element ever reported the fact and the conservative rule downstream redacted every typed value in the trace, the prompt and the log. The XML still carries it: re-read it once per settled snapshot and state the fact on the text fields it matches. A field it cannot match stays unstated, which still reads as a credential.
Four places said android reports the fact for nothing and that every typed value there is redacted. The sidecar now states it, so they described the old behaviour.
collectTextFields matched on class only, so a node the go side calls editable off its hintText was left unstated and its typed value redacted.
… fields Both derive the fact from a widget type a compose app never has, so the value reaches the trace in the clear. Verified on folio on both targets.
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
1 participant
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.
android was redacting every typed value, not just passwords. turns out maestro's tree mapper copies a fixed attribute list off the device xml and password isn't on it, so nothing on android ever reported the fact and the conservative rule redacted everything. sidecar now re-reads that xml once per settled snapshot and puts the fact back on the text fields it can match, anything it can't match stays redacted.
also the step log was useless. it now says screen, action, target and the typed value (through the same redaction), and screen isn't empty on native any more, it falls back to the *Screen route marker in the tree.
verified on all three targets with real folio runs. android: password redacted, email/account name/amount in clear. ios and web: screen name and log format fine.
one thing i found and did not fix, written up in docs/manual/runs.md: ios and web both state secure:false for a compose password field because they derive it from a widget type compose never has, so the password lands in the trace in clear text there. separate change.