mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 11:07:10 +00:00
redact passwords only, and say what each step did in the log (#93)
* feat(hierarchy): name the route a native tree shows 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. * feat(runner): say what each step did in the step log 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. * fix(sidecar): state on android whether a field is a secure entry 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. * docs: correct the record that android never reports a secure field 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. * test(sidecar): fail the build if maestro renames the call the fact comes from * fix(sidecar): state the fact on a field named by its hint alone collectTextFields matched on class only, so a node the go side calls editable off its hintText was left unstated and its typed value redacted. * docs: record that ios and web state secure:false for compose password 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. * feat(android): read the application id out of an apk parses the compiled AndroidManifest.xml rather than shelling out to aapt2, which lives in the versioned build-tools directory that hosts with only platform-tools never install. Claude-Session: https://claude.ai/code/session_012PVErdr3ZzyUASeVQDWsUc * feat(cli): let --android-app-path supply the bundle id --bundle-id stays required everywhere else, and an explicit one still wins, so the apk can never quietly override what was asked for. Claude-Session: https://claude.ai/code/session_012PVErdr3ZzyUASeVQDWsUc * docs: record that the apk can name the package itself Claude-Session: https://claude.ai/code/session_012PVErdr3ZzyUASeVQDWsUc * fix(testrun): pass the jvm the flag that silences the jdk 24 unsafe warning * feat(folio): ask which android device to run on when none is named * feat(folio): pin ios recipes to one simulator udid and ask when several match * docs(ci): say how just ios lands on the simulator the boot step chose * chore(folio): ignore run output anywhere under examples/folio * refactor(hierarchy): name no screen for a tree Transitional calls a cross-fade ScreenName kept its own reading of the route markers and disagreed with Transitional on a marker repeated by a nested node: it named the screen on a step the runner was skipping as unsettled. One reading now. * refactor(sidecar): inline the one attempt passed to callViewHierarchy A named constant and its own comment for a literal used once. * test(sidecar): compare the whole tree when checking the annotation changes nothing else The old assertions checked one id string and one bounds value, and passed with every other attribute stripped off every node. Now the annotated tree minus the two facts it stated must equal the input. * fix(sidecar): match a field to its xml node by class as well as id and bounds A wrapper drawn to the same bounds as the untagged field inside it shared the field's key, both were dropped as ambiguous, and every value typed into an untagged field was redacted. The class tells them apart. * docs(runs): record what the android hierarchy re-read costs per step Two 1m runs per binary on folio, same seed, before and after the re-read.
This commit is contained in:
25 files changed
+1287
-98
No files matched your search
@@ -948,6 +948,105 @@ internal fun treeWithoutKeyboard(
|
||||
return current
|
||||
}
|
||||
|
||||
// withSecureFacts states, on every text field in the tree, whether the platform
|
||||
// calls it a secure entry. maestro's host-side mapper copies a fixed attribute
|
||||
// list off the device's XML and `password` is not on it, so the tree alone
|
||||
// cannot tell a password field from a search box and everything typed anywhere
|
||||
// gets recorded as a credential. The XML the same read produced does carry the
|
||||
// fact, so it is fetched (lazily: a screen with no text field never pays for
|
||||
// it) and matched back onto the fields by identity, class and bounds.
|
||||
//
|
||||
// A field the XML cannot be matched to is left unstated rather than guessed.
|
||||
// Unstated reads as "may be a credential" downstream, which is the safe way to
|
||||
// be wrong.
|
||||
internal fun withSecureFacts(
|
||||
treeJson: String,
|
||||
viewHierarchyXml: () -> String?,
|
||||
): String {
|
||||
val root = try {
|
||||
jsonMapper.readTree(treeJson)
|
||||
} catch (_: Exception) {
|
||||
return treeJson
|
||||
}
|
||||
val fields = mutableListOf<com.fasterxml.jackson.databind.node.ObjectNode>()
|
||||
collectTextFields(root, fields)
|
||||
if (fields.isEmpty()) return treeJson
|
||||
val facts = secureFactsFromXml(viewHierarchyXml() ?: return treeJson)
|
||||
var stated = false
|
||||
for (field in fields) {
|
||||
val key = secureFactKey(
|
||||
nodeAttribute(field, "resource-id"),
|
||||
nodeAttribute(field, "class"),
|
||||
nodeAttribute(field, "bounds"),
|
||||
)
|
||||
field.put("secure", facts[key] ?: continue)
|
||||
stated = true
|
||||
}
|
||||
return if (stated) jsonMapper.writeValueAsString(root) else treeJson
|
||||
}
|
||||
|
||||
// secureFactsFromXml reads the password attribute uiautomator states on every
|
||||
// node. A key two nodes share answers for neither, so it is dropped: matching
|
||||
// the wrong node is how a credential ends up recorded in the clear.
|
||||
internal fun secureFactsFromXml(xml: String): Map<String, Boolean> {
|
||||
val document = try {
|
||||
javax.xml.parsers.DocumentBuilderFactory.newInstance()
|
||||
.newDocumentBuilder()
|
||||
.parse(java.io.ByteArrayInputStream(xml.toByteArray()))
|
||||
} catch (_: Exception) {
|
||||
return emptyMap()
|
||||
}
|
||||
val facts = mutableMapOf<String, Boolean>()
|
||||
val shared = mutableSetOf<String>()
|
||||
val nodes = document.getElementsByTagName("node")
|
||||
for (index in 0 until nodes.length) {
|
||||
val element = nodes.item(index) as? org.w3c.dom.Element ?: continue
|
||||
val key = secureFactKey(
|
||||
element.getAttribute("resource-id"),
|
||||
element.getAttribute("class"),
|
||||
element.getAttribute("bounds"),
|
||||
)
|
||||
val password = element.getAttribute("password") == "true"
|
||||
if (facts.put(key, password) != null) shared.add(key)
|
||||
}
|
||||
shared.forEach(facts::remove)
|
||||
return facts
|
||||
}
|
||||
|
||||
private fun secureFactKey(id: String, className: String, bounds: String) =
|
||||
"$id@$className@$bounds"
|
||||
|
||||
// A text field is what the Go side calls editable off the same two attributes
|
||||
// (internal/hierarchy): stating the fact on a narrower set would leave fields
|
||||
// the rest of the system treats as typeable answering for nothing.
|
||||
private fun collectTextFields(
|
||||
node: com.fasterxml.jackson.databind.JsonNode,
|
||||
into: MutableList<com.fasterxml.jackson.databind.node.ObjectNode>,
|
||||
) {
|
||||
if (node is com.fasterxml.jackson.databind.node.ObjectNode &&
|
||||
(
|
||||
nodeAttribute(node, "class").contains("EditText") ||
|
||||
nodeAttribute(node, "hintText").isNotEmpty()
|
||||
)
|
||||
) {
|
||||
into.add(node)
|
||||
}
|
||||
val children = node.get("children")
|
||||
if (children != null && children.isArray) {
|
||||
for (child in children) collectTextFields(child, into)
|
||||
}
|
||||
}
|
||||
|
||||
private fun nodeAttribute(
|
||||
node: com.fasterxml.jackson.databind.JsonNode,
|
||||
key: String,
|
||||
): String {
|
||||
val attributes = node.get("attributes") ?: return ""
|
||||
if (!attributes.isObject) return ""
|
||||
val value = attributes.get(key) ?: return ""
|
||||
return if (value.isNull) "" else value.asText()
|
||||
}
|
||||
|
||||
// SELECT_ALL_COMMAND selects the focused field's whole content with
|
||||
// CTRL+A (keycodes 113 and 29) and DELETE_KEY_COMMAND then deletes the
|
||||
// selection (keycode 67). Two key events, whatever the field holds.
|
||||
@@ -1162,7 +1261,11 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
|
||||
// can tell the runner the gesture reached nothing.
|
||||
private fun requireOnScreen(x: Int, y: Int) {
|
||||
val cached = extent
|
||||
if (cached != null && !offScreen(x, y, cached.first, cached.second)) return
|
||||
if (cached != null &&
|
||||
!offScreen(x, y, cached.first, cached.second)
|
||||
) {
|
||||
return
|
||||
}
|
||||
val info = driver.deviceInfo()
|
||||
val fresh = Pair(info.widthPixels, info.heightPixels)
|
||||
extent = fresh
|
||||
@@ -1302,13 +1405,40 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
|
||||
// not: it fetched the hierarchy ~4 more times on every mutating step. The
|
||||
// keyboard leg costs nothing either when no IME is standing in the tree,
|
||||
// which is what lets the Hierarchy RPC serve this too.
|
||||
override fun snapshotTree(): String = treeWithoutKeyboard(
|
||||
awaitSettledTree { hierarchy() },
|
||||
imePackage,
|
||||
dismiss = { runCatching { dadb.shell("input keyevent 4") } },
|
||||
reread = { awaitSettledTree { hierarchy() } },
|
||||
override fun snapshotTree(): String = withSecureFacts(
|
||||
treeWithoutKeyboard(
|
||||
awaitSettledTree { hierarchy() },
|
||||
imePackage,
|
||||
dismiss = { runCatching { dadb.shell("input keyevent 4") } },
|
||||
reread = { awaitSettledTree { hierarchy() } },
|
||||
),
|
||||
::deviceViewHierarchyXml,
|
||||
)
|
||||
|
||||
// deviceViewHierarchyXml re-reads the device tree in the form maestro parsed
|
||||
// it from, which is the only form still carrying the password attribute
|
||||
// maestro's own mapper drops. The call is private to maestro, so a version
|
||||
// that renames it leaves every typed value redacted rather than exposed;
|
||||
// the warning is what says that happened. The 1 is the attempt count,
|
||||
// maestro's own default for the call.
|
||||
private fun deviceViewHierarchyXml(): String? = runCatching {
|
||||
val attempts = Int::class.javaPrimitiveType
|
||||
val call = maestro.drivers.AndroidDriver::class.java
|
||||
.getDeclaredMethod("callViewHierarchy", attempts)
|
||||
call.isAccessible = true
|
||||
val response = call.invoke(driver, 1)
|
||||
response.javaClass.getMethod("getHierarchy").invoke(response) as String
|
||||
}.onFailure {
|
||||
if (viewHierarchyXmlWarned.compareAndSet(false, true)) {
|
||||
System.err.println(
|
||||
"warn: view hierarchy unreadable; typed values redacted: $it",
|
||||
)
|
||||
}
|
||||
}.getOrNull()
|
||||
|
||||
private val viewHierarchyXmlWarned =
|
||||
java.util.concurrent.atomic.AtomicBoolean(false)
|
||||
|
||||
override fun waitForIdle(durationMillis: Long) {
|
||||
// waitForAppToSettle blocks on the View-system animation and maestro's
|
||||
// own structural settle. It cannot see a Compose cross-fade: the fade
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
package dev.sanderling.sidecar
|
||||
|
||||
import org.junit.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
|
||||
// The tree maestro hands back for a login form: it names both fields and says
|
||||
// nothing about either being a credential entry, because maestro's mapper does
|
||||
// not copy the password attribute off the device's XML.
|
||||
private const val LOGIN_TREE = """
|
||||
{"attributes":{"bounds":"[0,0,1080,2340]"},"children":[
|
||||
{"attributes":{"resource-id":"LoginEmail","class":"android.widget.EditText","bounds":"[51,130][429,202]"},"children":[]},
|
||||
{"attributes":{"resource-id":"LoginPassword","class":"android.widget.EditText","bounds":"[51,249][429,321]"},"children":[]},
|
||||
{"attributes":{"resource-id":"LoginSubmit","class":"android.widget.Button","bounds":"[51,400][429,470]"},"children":[]}
|
||||
]}
|
||||
"""
|
||||
|
||||
// The same screen as the device reports it, where the fact still exists.
|
||||
private const val LOGIN_XML = """<?xml version='1.0' encoding='UTF-8'?>
|
||||
<hierarchy rotation="0">
|
||||
<node index="0" resource-id="" class="android.widget.FrameLayout" password="false" bounds="[0,0,1080,2340]">
|
||||
<node index="0" resource-id="LoginEmail" class="android.widget.EditText" password="false" bounds="[51,130][429,202]" />
|
||||
<node index="1" resource-id="LoginPassword" class="android.widget.EditText" password="true" bounds="[51,249][429,321]" />
|
||||
<node index="2" resource-id="LoginSubmit" class="android.widget.Button" password="false" bounds="[51,400][429,470]" />
|
||||
</node>
|
||||
</hierarchy>
|
||||
"""
|
||||
|
||||
private fun secureOf(tree: String, resourceId: String): Boolean? {
|
||||
val field = jacksonTree(tree, "resource-id", resourceId) ?: return null
|
||||
val secure = field.get("secure") ?: return null
|
||||
return secure.asBoolean()
|
||||
}
|
||||
|
||||
private fun jacksonTree(
|
||||
tree: String,
|
||||
attribute: String,
|
||||
value: String,
|
||||
): com.fasterxml.jackson.databind.JsonNode? {
|
||||
val mapper = com.fasterxml.jackson.module.kotlin.jacksonObjectMapper()
|
||||
fun walk(
|
||||
node: com.fasterxml.jackson.databind.JsonNode,
|
||||
): com.fasterxml.jackson.databind.JsonNode? {
|
||||
if (node.get("attributes")?.get(attribute)?.asText() == value) {
|
||||
return node
|
||||
}
|
||||
node.get("children")?.forEach { child ->
|
||||
walk(child)?.let { return it }
|
||||
}
|
||||
return null
|
||||
}
|
||||
return walk(mapper.readTree(tree))
|
||||
}
|
||||
|
||||
class SecureFactsTest {
|
||||
|
||||
// Without this, a password field and a search box look identical in the
|
||||
// tree, and everything downstream that records a typed value has to treat
|
||||
// every field as a credential: the whole run reads "[redacted]".
|
||||
@Test fun aPasswordFieldIsToldApartFromTheFieldBesideIt() {
|
||||
val annotated = withSecureFacts(LOGIN_TREE) { LOGIN_XML }
|
||||
|
||||
assertEquals(true, secureOf(annotated, "LoginPassword"))
|
||||
assertEquals(false, secureOf(annotated, "LoginEmail"))
|
||||
}
|
||||
|
||||
// Only what a value can be typed into needs the fact, and stating it on a
|
||||
// button would be stating it about something the platform never reported.
|
||||
@Test fun aNonEditableNodeIsLeftAlone() {
|
||||
assertNull(
|
||||
secureOf(
|
||||
withSecureFacts(LOGIN_TREE) {
|
||||
LOGIN_XML
|
||||
},
|
||||
"LoginSubmit",
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
// The Go side calls a node editable off its class or its hint, and a field
|
||||
// it will happily type into has to be a field this can speak for.
|
||||
@Test fun aFieldNamedByItsHintAloneIsStatedToo() {
|
||||
val hinted = """
|
||||
{"attributes":{"bounds":"[0,0,1080,2340]"},"children":[
|
||||
{"attributes":{"resource-id":"Search","class":"android.view.View","hintText":"Search","bounds":"[10,10,200,50]"},"children":[]}
|
||||
]}
|
||||
"""
|
||||
val xml = """<?xml version='1.0' encoding='UTF-8'?>
|
||||
<hierarchy rotation="0">
|
||||
<node index="0" resource-id="Search" class="android.view.View" password="false" bounds="[10,10,200,50]" />
|
||||
</hierarchy>
|
||||
"""
|
||||
|
||||
assertEquals(false, secureOf(withSecureFacts(hinted) { xml }, "Search"))
|
||||
}
|
||||
|
||||
// Unstated means "may be a credential" downstream. Every way this can fail
|
||||
// has to land there rather than on a false "not secure".
|
||||
@Test fun aFactThatCannotBeReadIsLeftUnstated() {
|
||||
for (xml in listOf<String?>(null, "", "<hierarchy", "<hierarchy/>")) {
|
||||
val annotated = withSecureFacts(LOGIN_TREE) { xml }
|
||||
assertNull(
|
||||
secureOf(annotated, "LoginPassword"),
|
||||
"xml ${xml ?: "null"}",
|
||||
)
|
||||
assertNull(
|
||||
secureOf(annotated, "LoginEmail"),
|
||||
"xml ${xml ?: "null"}",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// A wrapper drawn to the same bounds as the untagged field inside it shares
|
||||
// the field's empty id and its bounds. The class is what still tells them
|
||||
// apart; without it the two collide, the field answers for nothing, and
|
||||
// every value typed into an untagged field is redacted.
|
||||
@Test fun anUntaggedFieldInsideAWrapperOfItsOwnSizeIsStillStated() {
|
||||
val wrapped = """
|
||||
{"attributes":{"bounds":"[0,0,1080,2340]"},"children":[
|
||||
{"attributes":{"class":"android.view.View","bounds":"[51,249][429,321]"},"children":[
|
||||
{"attributes":{"class":"android.widget.EditText","bounds":"[51,249][429,321]"},"children":[]}
|
||||
]}
|
||||
]}
|
||||
"""
|
||||
val xml = """<?xml version='1.0' encoding='UTF-8'?>
|
||||
<hierarchy rotation="0">
|
||||
<node index="0" resource-id="" class="android.view.View" password="false" bounds="[51,249][429,321]">
|
||||
<node index="0" resource-id="" class="android.widget.EditText" password="true" bounds="[51,249][429,321]" />
|
||||
</node>
|
||||
</hierarchy>
|
||||
"""
|
||||
|
||||
val field = jacksonTree(
|
||||
withSecureFacts(wrapped) { xml },
|
||||
"class",
|
||||
"android.widget.EditText",
|
||||
)
|
||||
|
||||
assertEquals(true, field?.get("secure")?.asBoolean())
|
||||
}
|
||||
|
||||
// Two nodes sharing a key answer for neither: taking the first would state
|
||||
// "not secure" about a field that may be the other one.
|
||||
@Test fun anAmbiguousMatchIsLeftUnstated() {
|
||||
val duplicated = """<?xml version='1.0' encoding='UTF-8'?>
|
||||
<hierarchy rotation="0">
|
||||
<node index="0" resource-id="LoginPassword" class="android.widget.EditText" password="false" bounds="[51,249][429,321]" />
|
||||
<node index="1" resource-id="LoginPassword" class="android.widget.EditText" password="true" bounds="[51,249][429,321]" />
|
||||
</hierarchy>
|
||||
"""
|
||||
assertNull(
|
||||
secureOf(
|
||||
withSecureFacts(LOGIN_TREE) {
|
||||
duplicated
|
||||
},
|
||||
"LoginPassword",
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
// A screen with nothing to type into must not pay a device read for an
|
||||
// answer no field is waiting on.
|
||||
@Test fun aScreenWithNoTextFieldNeverReadsTheDevice() {
|
||||
val listTree =
|
||||
"""{"attributes":{"resource-id":"HomeScreen"},"children":[]}"""
|
||||
var reads = 0
|
||||
|
||||
val annotated = withSecureFacts(listTree) {
|
||||
reads++
|
||||
LOGIN_XML
|
||||
}
|
||||
|
||||
assertEquals(0, reads)
|
||||
assertEquals(listTree, annotated)
|
||||
}
|
||||
|
||||
// The fact is read through a call maestro keeps private, so a maestro
|
||||
// upgrade that renames it would leave every typed value redacted again with
|
||||
// nothing failing. This fails the build instead.
|
||||
@Test fun maestroStillExposesTheCallTheDeviceXmlComesFrom() {
|
||||
val call = maestro.drivers.AndroidDriver::class.java
|
||||
.getDeclaredMethod(
|
||||
"callViewHierarchy",
|
||||
Int::class.javaPrimitiveType,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
String::class.java,
|
||||
call.returnType.getMethod("getHierarchy").returnType,
|
||||
)
|
||||
}
|
||||
|
||||
// The rest of the tree has to survive the annotation: it is the same tree
|
||||
// every selector, bounds read and screen classification runs against.
|
||||
@Test fun theTreeIsOtherwiseUnchanged() {
|
||||
val mapper = com.fasterxml.jackson.module.kotlin.jacksonObjectMapper()
|
||||
val annotated = mapper.readTree(
|
||||
withSecureFacts(LOGIN_TREE) {
|
||||
LOGIN_XML
|
||||
},
|
||||
)
|
||||
|
||||
val stated = annotated.findParents("secure")
|
||||
assertEquals(2, stated.size)
|
||||
for (node in stated) {
|
||||
(node as com.fasterxml.jackson.databind.node.ObjectNode)
|
||||
.remove("secure")
|
||||
}
|
||||
assertEquals(mapper.readTree(LOGIN_TREE), annotated)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user