Peel tag and restore flags off settings rows

A dumpsys settings row can end in `tag:` and, on some vendor builds, in
`isValuePreservedInRestore:` or a bare `notPreservedInRestore` token.
The parser only peeled `default:` and `defaultSystemSet:` off the end
of a record, so on a row without a default those tokens stayed inside
the value, and on a row with one they landed in `defaultSystemSet`. A
value of `0 tag:null` is not the safe value `0`, so a setting at its
safe value was reported as dangerous; these are the false positives
measured in #912.

The metadata keys are ordered fields like the rest of the record, so
`_split_fields` reads them now, which also replaces the separate
handling of the default. The bare token has no `key:` shape and is
printed last, so it is stripped first and recorded as
`isValuePreservedInRestore: false`.

The bugreport fixture gains the three row shapes from #912: a restore
flag after `defaultSystemSet:`, and a `tag:` or a bare token directly
after the value. Two of them sit on dangerous settings at their safe
value, so the unchanged alert count of one is the false-positive check.

src/mvt/android/artifacts/settings.py

@@ -66,8 +66,20 @@ ANDROID_DANGEROUS_SETTINGS = [
 ]
 
 # dumpsys prints the fields of a setting record, and of a change history entry,
-# always in this order and separated by a single space.
-SETTING_FIELDS = ("_id", "name", "pkg", "value")
+# always in this order and separated by a single space. After the value come
+# `default:` and `defaultSystemSet:` when a default is recorded, then `tag:`;
+# some vendor builds add whether the value survives a restore, either as
+# `isValuePreservedInRestore:` or as a bare `notPreservedInRestore` token.
+SETTING_FIELDS = (
+    "_id",
+    "name",
+    "pkg",
+    "value",
+    "default",
+    "defaultSystemSet",
+    "tag",
+    "isValuePreservedInRestore",
+)
 HISTORY_FIELDS = ("time", "mode", "oldValue", "newValue", "package")
 
 NAMESPACE_PATTERN = re.compile(
@@ -81,9 +93,9 @@ class Settings(AndroidArtifact):
 
     Every row of the settings provider becomes one result, keeping the fields
     dumpsys prints alongside the value: the row id, the package which recorded
-    the setting, the default, and the change history. A setting name can appear
-    more than once within a namespace, so results are a list rather than a
-    mapping.
+    the setting, the default, the tag, and the change history. A setting name
+    can appear more than once within a namespace, so results are a list rather
+    than a mapping.
     """
 
     def serialize(self, result: ModuleAtomicResult) -> ModuleSerializedResult:
@@ -269,26 +281,14 @@ class Settings(AndroidArtifact):
         section_end: Optional[datetime],
     ) -> ModuleAtomicResult:
         text = "\n".join(record_lines).rstrip()
-
-        # `default:` and `defaultSystemSet:` are printed after the value, and
-        # the default may itself be multi-line, so peel them off the end first.
-        default = None
-        default_system_set = None
-        head, separator, tail = text.rpartition(" defaultSystemSet:")
-        if separator:
-            default_system_set = tail.strip()
-            text = head
-            head, separator, tail = text.rpartition(" default:")
-            if separator:
-                default = tail
-                text = head
+        # The bare `notPreservedInRestore` token has no `key:` shape and is
+        # printed last, so peel it off before splitting the fields.
+        head = text.removesuffix(" notPreservedInRestore")
 
         record: ModuleAtomicResult = {"namespace": namespace, "user": user}
-        record.update(self._split_fields(text, SETTING_FIELDS))
-        if default is not None:
-            record["default"] = default
-        if default_system_set is not None:
-            record["defaultSystemSet"] = default_system_set
+        record.update(self._split_fields(head, SETTING_FIELDS))
+        if head != text:
+            record["isValuePreservedInRestore"] = "false"
 
         record["history"] = [
             self._parse_history(entry, section_end) for entry in history_lines

tests/android/test_artifact_settings.py

@@ -25,7 +25,7 @@ class TestSettingsArtifact:
     def test_parsing(self):
         settings = parse_bugreport_settings()
 
-        assert len(settings.results) == 11
+        assert len(settings.results) == 12
         assert {result["namespace"] for result in settings.results} == {
             "config",
             "global",
@@ -69,6 +69,24 @@ class TestSettingsArtifact:
         assert record["value"] == "com.example.dialer,com.example.camera"
         assert record["default"] == "com.android.settings,\n        com.android.vending"
 
+    def test_trailing_metadata_is_not_part_of_the_value(self):
+        settings = parse_bugreport_settings()
+
+        record = find(settings, "lock_screen_show_notifications")[0]
+        assert record["value"] == "1"
+        assert record["defaultSystemSet"] == "true"
+        assert record["isValuePreservedInRestore"] == "true"
+
+        # Without a default, the tag or the restore token follows the value.
+        record = find(settings, "accessibility_enabled")[1]
+        assert record["value"] == "0"
+        assert record["tag"] == "null"
+        assert "default" not in record
+
+        record = find(settings, "send_action_app_error")[0]
+        assert record["value"] == "1"
+        assert record["isValuePreservedInRestore"] == "false"
+
     def test_repeated_names_are_kept_as_separate_records(self):
         settings = parse_bugreport_settings()
 

tests/android_bugreport/test_bugreport.py

@@ -96,7 +96,7 @@ class TestBugreportAnalysis:
 
     def test_settings_module(self):
         m = self.launch_bug_report_module(Settings)
-        assert len(m.results) == 11
+        assert len(m.results) == 12
 
         assert len(m.alertstore.alerts) == 1
         assert "accessibility_enabled = 1" in m.alertstore.alerts[0].message

tests/artifacts/android_data/bugreport/dumpstate.txt

@@ -280,6 +280,7 @@ com.example.chat
 GLOBAL SETTINGS (user 0)
 _id:2070 name:adb_wifi_enabled pkg:android value:0 default:0 defaultSystemSet:true
 _id:778 name:hidden_api_blacklist_exemptions value:{null}
+_id:9640 name:send_action_app_error pkg:android value:1 notPreservedInRestore
 _id:9631 name:development_settings_enabled pkg:com.android.settings value:1 default:1 defaultSystemSet:true
 	History (development_settings_enabled)
 		time:11-02 11:21:22.212 mode:update oldValue:null newValue:1 package:com.android.settings
@@ -299,12 +300,12 @@ _id:41654 name:widget_instance_data pkg:com.android.systemui value:{
 } defaultSystemSet:true
 
 SECURE SETTINGS (user 0)
-_id:907 name:lock_screen_show_notifications pkg:com.android.settings value:1 default:1 defaultSystemSet:true
+_id:907 name:lock_screen_show_notifications pkg:com.android.settings value:1 default:1 defaultSystemSet:true isValuePreservedInRestore:true
 _id:240 name:accessibility_enabled pkg:android value:1 default:0 defaultSystemSet:true
 	History (accessibility_enabled)
 		time:03-28 22:41:07.980 mode:update oldValue:0 newValue:1 package:com.example.helper
 
 SECURE SETTINGS (user 10)
-_id:311 name:accessibility_enabled pkg:android value:0 default:0 defaultSystemSet:true
+_id:311 name:accessibility_enabled pkg:android value:0 tag:null
 
 --------- 0.019s was the duration of dumpsys settings, ending at: 2022-03-29 23:14:28