fix(intents): an ADB-off entry never scans, and a truncated detail is never believed (t21)
The t18 review's three findings, closed with the reviewer's own repro.
F1 (HIGH — the harm was real shells, not a wrong label). `async_scan_intents`
decided between `adb_disabled` and `adb_unavailable` INSIDE `if self._poller is
None:`, so a manager that HELD a poller fell through to the shell loop: with
`adb_enabled: False` and a connected poller the review measured status `fresh`,
one candidate CACHED, and `pm dump com.x …` really sent. The option is a fact
about the ENTRY, so it is now checked once at the top and returns before
anything else — no cache read, no cache write, no shell. The no-poller branch
keeps the OTHER honest answer: with the option ON and nobody to ask, the status
is `adb_unavailable` ("wait or reconnect"), never `adb_disabled` ("go turn the
option on"), because the two have different fixes. Five tests pin it, including
a cache-leak case (a manager with a cached answer whose option is then switched
off answers `adb_disabled`, not the cache) and both statuses with a FORCED scan.
F2 (low, but it fabricated a value that looked real). A capture cut mid-detail
produced `Scheme: "ht` → scheme `"ht` and `Authority: "www.exam` → that
authority, and `_candidate_data` would then prefill the form with a URL no app
can match. Not an injection (everything stays quoted when used), which is
exactly why nothing caught it. `_unquote` now DROPS a value that opens a quote
and never closes it (a lone quote too), so the callers turn it into None: the
candidate survives with its intact action/component and simply carries no
scheme/authority/path — and therefore prefills NO data instead of a broken URL.
A complete quoted value is still read (the anti-overcorrection pin), and the
consequence is pinned at the consumption end too, through the real options-flow
helper.
F3 (low). The floor comment now answers the review in writing, where the next
reader looks: the t18 review's measurement (awaited_calls 6 against a floor of 5,
three other counters within 2) is named, and the verdict is recorded — those ARE
the policy margins (2 for owner-counters because an owner really can be
refactored away, 1 for awaited_calls because six sites cannot carry a percentage
band). The counters were genuinely updated, not dodged: raised to the real sweep
when the code grew (20→24, 70→75, 235→275, 55→63, 1400→1620) and re-derived
twice since (t20 1620→1615, t21 1615→1617 for this unit's blocks).
EVIDENCE THAT THE TESTS PIN THE FIX (not merely pass): the same files were run
against the PRE-fix source (git archive HEAD, a throwaway copy under /tmp) — 9 of
the 11 new tests FAIL there, including the reviewer's repro, which fails with
`assert 'fresh' == 'adb_disabled'`. The two that pass are the deliberate
anti-overcorrection pins: a COMPLETE quoted value is still parsed, and the
no-poller control keeps its own status.
NO REGRESSION: 900 passed / 5 skipped, 901 / 4 with androidtvremote2; the
injection proof, the fixture's candidates with dedup/flagging/order/bound
disclosures, both cache outcomes, the registration round trip through the real
FlowManager, button lifecycle and defaults, the YAML seed rule and §7.1
(supports_response optional, a real return_response call, a no-response call) all
still pass. The strings files are byte-identical (untouched here). Real-HA pattern
collected and RAN 3 files → 74 passed / 0 skipped. Drift exit 0 (1650 blocks).
NOT VERIFIED HERE: the device side remains a scripted double — no real Shield was
asked for its resolver table, so the shell COUNT and the status vocabulary are
what is proven, not real firmware behaviour. No browser either: the form prefill
is asserted on the values the helper returns.
This commit is contained in:
1 parent
6b866019ef
commit
8f064b3247
5 files changed
+264
-16
No files matched your search
@@ -404,6 +404,100 @@ class TestRankingAndHonesty:
|
||||
assert is_likely_internal(None, "com.x/ConstraintProxy$Foo") is True
|
||||
|
||||
|
||||
class TestATruncatedValueIsNotFabricated:
|
||||
"""t18/F2: a capture cut mid-value must not yield a PARTIAL value.
|
||||
|
||||
Android prints quoted detail values (``Scheme: "https"``, ``Authority:
|
||||
"www.netflix.com": -1``, ``Path: "PatternMatcher{GLOB: /title.*}"``). A dump
|
||||
truncated inside one of them used to produce ``"ht`` / ``"www.exam`` — values
|
||||
that LOOK real, which ``_candidate_data`` would then turn into a URL the app
|
||||
cannot match. Not an injection (everything stays quoted when used), which is
|
||||
exactly why nothing else caught it: the corrupt value must be distinguishable
|
||||
from a real one, so an unterminated quoted value is DROPPED.
|
||||
"""
|
||||
|
||||
#: Every quoted detail cut before its closing quote — the reviewer's cases,
|
||||
#: plus a cut in the middle of the Path matcher.
|
||||
TRUNCATED = (
|
||||
"Activity Resolver Table:\n"
|
||||
" Schemes:\n"
|
||||
" https:\n"
|
||||
" deca7fa com.netflix.ninja/.MainActivity filter fc1\n"
|
||||
' Action: "android.intent.action.VIEW"\n'
|
||||
' Scheme: "ht\n'
|
||||
' Authority: "www.exam\n'
|
||||
' Path: "PatternMatcher{GLOB: /title\n'
|
||||
"Service Resolver Table:\n"
|
||||
)
|
||||
|
||||
def test_the_partial_values_are_dropped_not_kept(self):
|
||||
[candidate] = parse_activity_resolver_table(self.TRUNCATED)
|
||||
|
||||
assert candidate.scheme is None, "a half scheme is not a scheme"
|
||||
assert candidate.authority is None, "a half host is not a host"
|
||||
assert candidate.path is None, "a half pattern is not a path"
|
||||
assert candidate.path_kind is None
|
||||
|
||||
def test_the_intact_details_survive_and_nothing_carries_a_quote(self):
|
||||
[candidate] = parse_activity_resolver_table(self.TRUNCATED)
|
||||
|
||||
assert candidate.action == "android.intent.action.VIEW", (
|
||||
"the value that WAS complete is still read"
|
||||
)
|
||||
assert candidate.component == "com.netflix.ninja/.MainActivity"
|
||||
for value in (candidate.scheme, candidate.authority, candidate.path):
|
||||
assert value is None or not value.startswith(("'", '"')), value
|
||||
|
||||
def test_an_unterminated_value_at_the_very_end_of_the_input(self):
|
||||
"""The last line can be the cut one — the end of input is not a terminator."""
|
||||
cut = (
|
||||
"Activity Resolver Table:\n"
|
||||
" Schemes:\n"
|
||||
" https:\n"
|
||||
" deca7fa com.netflix.ninja/.MainActivity filter fc1\n"
|
||||
' Scheme: "ht'
|
||||
)
|
||||
|
||||
[candidate] = parse_activity_resolver_table(cut)
|
||||
|
||||
assert candidate.scheme is None
|
||||
assert candidate.authority is None
|
||||
assert candidate.path is None
|
||||
|
||||
def test_a_lone_quote_is_dropped_too(self):
|
||||
cut = (
|
||||
"Activity Resolver Table:\n"
|
||||
" Schemes:\n"
|
||||
" https:\n"
|
||||
" deca7fa com.netflix.ninja/.MainActivity filter fc1\n"
|
||||
' Scheme: "\n'
|
||||
)
|
||||
|
||||
[candidate] = parse_activity_resolver_table(cut)
|
||||
|
||||
assert candidate.scheme is None
|
||||
|
||||
def test_a_complete_quoted_value_is_still_read(self):
|
||||
"""The anti-overcorrection pin: dropping must not eat good values."""
|
||||
intact = (
|
||||
"Activity Resolver Table:\n"
|
||||
" Schemes:\n"
|
||||
" https:\n"
|
||||
" deca7fa com.netflix.ninja/.MainActivity filter fc1\n"
|
||||
' Action: "android.intent.action.VIEW"\n'
|
||||
' Scheme: "https"\n'
|
||||
' Authority: "www.netflix.com": -1\n'
|
||||
' Path: "PatternMatcher{LITERAL, /browse}"\n'
|
||||
"Service Resolver Table:\n"
|
||||
)
|
||||
|
||||
[candidate] = parse_activity_resolver_table(intact)
|
||||
|
||||
assert candidate.scheme == "https"
|
||||
assert candidate.authority == "www.netflix.com"
|
||||
assert candidate.path == "/browse"
|
||||
assert candidate.path_kind == "LITERAL"
|
||||
|
||||
class TestDegradation:
|
||||
"""A shape we have never seen degrades to NO candidate, never a wrong one."""
|
||||
|
||||
@@ -848,3 +942,79 @@ class TestTheManagerScan:
|
||||
assert len(result.candidates) == INTENT_CANDIDATE_LIMIT
|
||||
assert result.truncated is True, "a capped answer never looks complete"
|
||||
assert manager.intent_cache.get(NETFLIX).truncated is True
|
||||
|
||||
|
||||
class TestAdbDisabledNeverTouchesTheDevice:
|
||||
"""t18/F1: an entry whose OPTION says ADB is off must not scan at all.
|
||||
|
||||
The review measured the defect: with ``adb_enabled: False`` and a CONNECTED
|
||||
poller, the scan fell straight through to the shell loop — ``pm dump com.x …``
|
||||
was really sent, one candidate was CACHED, and the status read ``fresh`` (a
|
||||
silent poller made it ``adb_unavailable``, so even the label depended on the
|
||||
wrong thing). "ADB is switched off" is a fact about the ENTRY, so the option is
|
||||
checked once at the top and returns before any shell or cache touch.
|
||||
"""
|
||||
|
||||
async def test_a_held_poller_does_not_override_the_option(self):
|
||||
poller = _IntentsPoller(default=NETFLIX_DUMP) # connected, would answer
|
||||
manager = _manager(poller, adb_enabled=False)
|
||||
|
||||
result = await manager.async_scan_intents(NETFLIX)
|
||||
|
||||
assert result.status == INTENT_SCAN_ADB_DISABLED
|
||||
assert result.answered is False
|
||||
assert result.candidates == ()
|
||||
assert poller.shell_calls == [], "the device was never touched"
|
||||
assert manager.intent_scan_state(NETFLIX) is None, "nothing was cached"
|
||||
|
||||
async def test_a_forced_scan_is_refused_the_same_way(self):
|
||||
poller = _IntentsPoller(default=NETFLIX_DUMP)
|
||||
manager = _manager(poller, adb_enabled=False)
|
||||
|
||||
assert (await manager.async_scan_intents(NETFLIX)).status == (
|
||||
INTENT_SCAN_ADB_DISABLED
|
||||
)
|
||||
forced = await manager.async_scan_intents(NETFLIX, force=True)
|
||||
|
||||
assert forced.status == INTENT_SCAN_ADB_DISABLED
|
||||
assert poller.shell_calls == [], "force does not override the option either"
|
||||
assert manager.intent_scan_state(NETFLIX) is None
|
||||
|
||||
async def test_a_silent_poller_cannot_change_the_status(self):
|
||||
"""The review's second half: the LABEL must come from the option."""
|
||||
silent_but_disabled = _manager(_IntentsPoller(default=None), adb_enabled=False)
|
||||
|
||||
assert (await silent_but_disabled.async_scan_intents(NETFLIX)).status == (
|
||||
INTENT_SCAN_ADB_DISABLED
|
||||
)
|
||||
assert silent_but_disabled._poller.shell_calls == []
|
||||
|
||||
# and the mirror image: ADB ON with a silent poller is "could not ask",
|
||||
# never "switched off" — the two have different fixes
|
||||
silent_but_enabled = _manager(_IntentsPoller(default=None), adb_enabled=True)
|
||||
assert (await silent_but_enabled.async_scan_intents(NETFLIX)).status == (
|
||||
INTENT_SCAN_ADB_UNAVAILABLE
|
||||
)
|
||||
|
||||
async def test_a_cached_answer_cannot_leak_out_of_a_disabled_entry(self):
|
||||
"""ADB off is answered BEFORE the cache is read — no stale prefill."""
|
||||
poller = _IntentsPoller(default=NETFLIX_DUMP)
|
||||
manager = _manager(poller)
|
||||
first = await manager.async_scan_intents(NETFLIX)
|
||||
assert first.status == INTENT_SCAN_FRESH and first.candidates
|
||||
|
||||
manager.entry.options[CONF_ADB_ENABLED] = False # the user turns ADB off
|
||||
after = await manager.async_scan_intents(NETFLIX)
|
||||
|
||||
assert after.status == INTENT_SCAN_ADB_DISABLED
|
||||
assert after.candidates == (), "a disabled entry offers no scan answer at all"
|
||||
assert len(poller.shell_calls) == 1, "only the original, permitted scan ran"
|
||||
|
||||
async def test_the_no_poller_control_keeps_its_own_status(self):
|
||||
manager = _manager(None, adb_enabled=True)
|
||||
|
||||
result = await manager.async_scan_intents(NETFLIX)
|
||||
|
||||
assert result.status == INTENT_SCAN_ADB_UNAVAILABLE, (
|
||||
"no poller is not the same fact as the option being off"
|
||||
)
|
||||
Reference in new issue
Block a user