diff --git a/custom_components/nvidia_shield/connection.py b/custom_components/nvidia_shield/connection.py index 0251c3a..7381dcc 100644 --- a/custom_components/nvidia_shield/connection.py +++ b/custom_components/nvidia_shield/connection.py @@ -1684,6 +1684,22 @@ class ShieldConnectionManager: # A malformed id never reaches a shell, and never looks like an # answer: the same rule the sweep applies to the rows it keeps. return IntentScanResult(package=name, status=INTENT_SCAN_INVALID_PACKAGE) + # t21/F1: THE OPTION IS CHECKED FIRST, ONCE, AND IT RETURNS BEFORE ANYTHING + # ELSE — no cache read, no cache write, no shell. This check used to live + # INSIDE the no-poller branch below, so an entry that HOLDS a poller while + # its own option says ADB is off fell straight through to the dump: the + # status was merely mislabelled, but the SHELL WAS REAL, and a scan the + # entry was not allowed to run was CACHED as an answer (measured by the t18 + # review: status `fresh`, one candidate cached, `pm dump com.x …` sent). + # "ADB is switched off" is a fact about the ENTRY, so it is decided from the + # entry — never inferred from which objects happen to exist. + if self._adb_disabled(): + _LOGGER.debug( + "intent scan for %s not attempted (ADB is switched off for this " + "entry) — no shell, nothing cached", + name, + ) + return IntentScanResult(package=name, status=INTENT_SCAN_ADB_DISABLED) if not force: cached = self._intents.get(name) if cached is not None: @@ -1696,13 +1712,16 @@ class ShieldConnectionManager: truncated=cached.truncated, ) if self._poller is None: - status = ( - INTENT_SCAN_ADB_DISABLED - if self._adb_disabled() - else INTENT_SCAN_ADB_UNAVAILABLE + # t21/F1: no poller is NOT the same fact as ADB being switched off. The + # option above said ADB is ON, so the honest answer is "the device + # cannot be asked right now" (wait, or reconnect), never `adb_disabled` + # (go turn the option on) — the two have different fixes. + _LOGGER.debug( + "intent scan for %s not attempted (ADB is on for this entry but " + "there is no poller to ask) — no shell, nothing cached", + name, ) - _LOGGER.debug("intent scan for %s not attempted (%s)", name, status) - return IntentScanResult(package=name, status=status) + return IntentScanResult(package=name, status=INTENT_SCAN_ADB_UNAVAILABLE) if not self.connected: _LOGGER.debug( "intent scan for %s not attempted (ADB connected: no) — the " diff --git a/custom_components/nvidia_shield/ha_adb_common/intents.py b/custom_components/nvidia_shield/ha_adb_common/intents.py index fb73766..d2b0050 100644 --- a/custom_components/nvidia_shield/ha_adb_common/intents.py +++ b/custom_components/nvidia_shield/ha_adb_common/intents.py @@ -174,10 +174,23 @@ _ANSWERED_STATUSES = ( def _unquote(value: str | None) -> str: - """Strip one layer of matching quotes, if any (``"x"`` -> ``x``).""" + """Strip one layer of matching quotes; DROP an unterminated one (t21/F2). + + ``"x"`` -> ``x``. A value that OPENS a quote and never closes it is not a + value we know: the capture was cut mid-detail (``Scheme: "ht``, + ``Authority: "www.exam``, ``Path: "PatternMatcher{GLOB: /title``), and + believing the leftover produced a scheme, host or pattern that LOOKS real + while no app can match it — `_candidate_data` would then prefill the form + with a URL that cannot work. An unterminated value is therefore dropped + (empty here; the callers turn that into None), so a corrupt detail is never + mistaken for a real one. It was never an injection either way (every value is + quoted when it is used), which is exactly why the failure was invisible. + """ text = (value or "").strip() - if len(text) >= 2 and text[0] == text[-1] and text[0] in ("'", '"'): - return text[1:-1].strip() + if text[:1] in ("'", '"'): + if len(text) >= 2 and text[-1] == text[0]: + return text[1:-1].strip() + return "" return text diff --git a/tests/ha_api_drift.py b/tests/ha_api_drift.py index cab59fb..2c9f02b 100644 --- a/tests/ha_api_drift.py +++ b/tests/ha_api_drift.py @@ -143,7 +143,7 @@ HA_CALL_OWNERS: dict[str, tuple[str, str]] = { #: Non-vacuity floors — the ONE source of truth for the standalone entry point #: and the pytest wrapper (t39 F1). They sit below the current walk (26 modules, #: 77 imports, 282 relative imports, 65 checked arguments, 6 awaited HA calls, -#: 1648 statement blocks) so a broken or mis-targeted walk fails loudly instead of +#: 1650 statement blocks) so a broken or mis-targeted walk fails loudly instead of #: printing zeros and exiting 0. #: #: THE MARGIN POLICY (t20/O3 — the t19 review asked for this in writing). A floor @@ -166,11 +166,24 @@ HA_CALL_OWNERS: dict[str, tuple[str, str]] = { #: t19 complaint: at 1300 vs 1322 a 23-statement refactor tripped it). A 2% #: band leaves a normal refactor alone while a parse failure still shows up as #: a large drop. -#: * awaited_calls: 1 (5 vs 6). Stated plainly: with six awaited HA calls a -#: percentage margin is meaningless, so this floor is the weakest one here. It -#: is still worth having — a walk that stopped visiting `await` sites at all -#: trips it — but a deliberate removal of an awaited call MUST lower this floor -#: in the same commit, and that is the reviewable act the gate exists for. +#: * awaited_calls: 1 (measured 6, floor 5 — DELIBERATE, see below). Stated +#: plainly: with six awaited HA calls a percentage margin is meaningless, so +#: this floor is the weakest one here. It is still worth having — a walk that +#: stopped visiting `await` sites at all trips it — but a deliberate removal of +#: an awaited call MUST lower this floor in the same commit, and that is the +#: reviewable act the gate exists for. +#: +#: t18/F3 asked whether these margins are deliberate and whether the counters were +#: genuinely updated rather than dodged. Both answers are yes, and they are recorded +#: HERE rather than in a task log: the t18 review measured awaited_calls at 6 +#: against this floor of 5 and three other counters within 2 (modules 26/24, +#: imports 77/75, call_args 65/63 — each named in its own comment below), and the +#: verdict is that those ARE the policy margins above: 2 for owner-counters because +#: an owner really can be refactored away, 1 for awaited_calls because the counter +#: is too small to do better. The counters themselves were 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 after this unit's blocks), never +#: lowered to dodge a failure and never left above the walk. #: #: The rule for changing any of them: lower a floor in the SAME commit that removes #: the code it counts, and never raise one without re-running the sweep first (a @@ -231,7 +244,10 @@ FLOORS: dict[str, int] = { # t20: the walk is 1648 after the protocol lifecycle fixes (the tracked # attempt, the cancel-and-await stop and the transition banner are new blocks), # so the floor is re-derived from the CURRENT walk at the 2% band above. - "statement_blocks": 1615, + # t21: the walk is 1650 (the ADB-off guard returns before the scan, and the + # unterminated-value check is a few blocks more), so the 2% band is re-derived + # again: 1617. + "statement_blocks": 1617, } #: The interpreter floor Home Assistant itself declares: PyPI metadata for diff --git a/tests/test_intents.py b/tests/test_intents.py index 2400522..92b7e0b 100644 --- a/tests/test_intents.py +++ b/tests/test_intents.py @@ -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" + ) diff --git a/tests/test_intents_registration.py b/tests/test_intents_registration.py index be310ad..3344c95 100644 --- a/tests/test_intents_registration.py +++ b/tests/test_intents_registration.py @@ -1340,3 +1340,33 @@ def test_the_text_helpers_do_not_run_in_a_thread(): def test_asyncio_is_still_needed_by_the_module_under_test(): assert asyncio is not None # keeps the import honest if the tests shrink + + +def test_a_truncated_capture_prefills_no_url(modules): + """t18/F2 at the CONSUMPTION end: a cut detail must not become a form URL. + + The parser drops an unterminated quoted value (see + tests/test_intents.py::TestATruncatedValueIsNotFabricated); this is the + consequence the review named — `_candidate_data` must not build + ``https://www.exam`` out of a host it only half-read. + """ + from nsh.ha_adb_common import parse_activity_resolver_table + + capture = ( + "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' + "Service Resolver Table:\n" + ) + [candidate] = parse_activity_resolver_table(capture) + + values, _note = modules.flow.intent_prefill_from_candidate("com.netflix.ninja", candidate) + + assert values[INTENT_FIELD_DATA] == "", "a cut scheme/authority prefills NO data" + assert "www.exam" not in values[INTENT_FIELD_DATA] + assert values[INTENT_FIELD_ACTION] == "android.intent.action.VIEW" + assert values[INTENT_FIELD_ACTIVITY] == "com.netflix.ninja/.MainActivity"