Allow for redacting/hiding sensitive test in `inputText` command
Proposed changes
- Add a
redactboolean field to theinputTextcommand - In the case it's set to
true, replace the value oftextto"[REDACTED]"in the following locations:- The console output when
--format=NOOP -
maestro.log -
commands-(<flow>).json
- The console output when
I chose to use redact instead of mask and [REDACTED] instead of XXXX as it seemed clearer but feel free to ask if you want it changed back.
Testing
I added 3 test cases:
- The JSON serialization of the
InputTextCommand(No deserialization though) - The correct parsing of the Yaml
redactfield - The correct input of the non-redacted text value
I ran the following commands:
- Run
./gradlew :maestro-test:test - Run
./gradlew :maestro-cli:test - Run
./gradlew :maestro-client:test - Run
./gradlew :maestro-orchestra:test - Run
./gradlew :maestro-orchestra-models:test
Manual tests
The tests below have been done manually. I tried seeing how to automate them, but ideally, this needs to be able to intercept the LOGGER calls in IntegrationTest.kt. Please advise if you want me to add tests for the cases below.
- Build the cli and then run
012_input_text.yaml- Run
./gradlew maestro-cli:installDist - Run
./maestro-cli/build/install/maestro/bin/maestro test maestro-test/src/test/resources/012_input_text.yaml - Check for the absence of the value "secret password"
- In the console output
- In
~/.maestro/tests/<timestamp>/maestro.log - In
~/.maestro/tests/<timestamp>/commands-(012_input_text.yaml).json
- Replace "secret password" with "💸" in
012_input_text.yaml- Run the flow
012_input_text.yamlon an Android device - Check for the absence of the value "💸" in the same locations (console output,
maestro.logandcommands-(012_input_text.yaml).json)
- Run the flow
- Run
Here are sample files with the results:
commands-(012_input_text.yaml).json maestro.log output.txt
With "💸" replacing "secret password" on Android :
commands-(012_input_text.yaml).json maestro.log output.txt
Issues fixed
Fixes #1226
Any chance we can get this pushed through given the sensitivity / security issues?
Any chance we can get this pushed through given the sensitivity / security issues?
Plus +1, it's also blocker for us due to security reasons.
I'm happy to merge this as soon as it's reviewed :)
@tokou Any idea why this is failing after the rebase?
Yes @Fishbowler, a lot of changes happened in the meantime (mostly related to AI I think) I need to rework the branch to fix these issues.