SEP-1807: Cover settings-override resolution and PATCH preservation (#1549)

Add regression coverage for nested-key resolution and PATCH credential
preservation in the shared settings-override registry. Test-only changes
guard against alias lookup failures and persisting mask literals instead
of live credentials.

- **Nested resolution:** Cover alias-only matches, unresolved keys,
multi-level and case-insensitive mapping traversal, and canonical
ancestor prefixes through `override_provenance_for_rows`, which replaced
`override_keys_for_rows`.
- **PATCH preservation:** Cover absent and recursively nested credential
URLs, secret-valued dictionaries and lists, and reordered model
collections with fallback item matching. Assert returned payloads, not
private predicates.
- **Coverage:** Add 16 cases across the two registry test files. The
requested suite passes all 577 tests; registry coverage increases from
87.05% to 94.18%. Pre-commit passes.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: yyyyyyyan <24644216+yyyyyyyan@users.noreply.github.com>

Author: Copilot

tests/app/core/settings_override/test_registry.py

@@ -13,13 +13,13 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-"""Tests for the HOT-classification registry."""
+"""Test settings-override classification, resolution, and PATCH preservation."""
 
 from string import Template
 from typing import ClassVar
 
 import pytest
-from pydantic import BaseModel, SecretStr
+from pydantic import BaseModel, HttpUrl, SecretStr
 from sqlmodel.ext.asyncio.session import AsyncSession
 
 from app.core.alerts.config import AlertSettings
@@ -43,12 +43,15 @@
     MaterializerPurpose,
     nested_overridable_field_names,
     override_rows_for_key,
+    preserve_credential_urls_in_model_payload,
     preserve_patch_credential_url_value,
+    preserve_secrets_in_model_payload,
     ReloadClassification,
     resolve_nested_field_metadata,
     SECRET_STR_MASK,
     unwrap_secrets_for_storage,
 )
+from app.core.utils.fields import CredentialHttpUrl, redact_credential_url
 from app.core.utils.pydantic import field_with_metadata
 from app.inventory.config import InventorySettings
 from app.sep.config import SEPSettings
@@ -200,6 +203,60 @@ def test_preserve_patch_credential_url_value_for_materializer_payload() -> None:
     assert preserved["endpoint"] == current["endpoint"]
 
 
+class _CredentialUrlModel(BaseModel):
+    """Represent a model with an optional credential-bearing endpoint."""
+
+    endpoint: CredentialHttpUrl | None = HttpUrl.build(
+        scheme="https",
+        username="test-user",
+        host="service.test",
+        password="synthetic-test-value",
+    )
+
+
+def test_preserve_credential_urls_with_none_current_leaf() -> None:
+    """Leave a masked URL unchanged when no live password exists to restore."""
+    current = _CredentialUrlModel(endpoint=None)
+    incoming = _CredentialUrlModel().model_dump(mode="json")
+
+    preserved = preserve_credential_urls_in_model_payload(
+        _CredentialUrlModel, current, incoming
+    )
+
+    assert preserved == incoming
+    assert preserved is not incoming
+
+
+def test_preserve_patch_credential_url_value_recurses_into_nested_model() -> None:
+    """Restore a nested URL password while retaining an outer-field PATCH."""
+
+    class _ConnectionGroup(BaseModel):
+        connection: _CredentialUrlModel
+        label: str
+
+    class _ConnectionSettings(BaseModel):
+        group: _ConnectionGroup
+
+    current = _ConnectionGroup(
+        connection=_CredentialUrlModel(),
+        label="original",
+    )
+    incoming = current.model_dump(mode="json")
+    incoming["label"] = "updated"
+
+    preserved = preserve_patch_credential_url_value(
+        _ConnectionSettings.model_fields["group"], current, incoming
+    )
+
+    assert preserved == {
+        "connection": {"endpoint": str(current.connection.endpoint)},
+        "label": "updated",
+    }
+    assert incoming["connection"]["endpoint"] == redact_credential_url(
+        str(current.connection.endpoint)
+    )
+
+
 class _SecretLeafModel(BaseModel):
     """Nested model with a scalar SecretStr leaf (PMM-shaped)."""
 
@@ -229,6 +286,131 @@ class _DictSecretSettings(BaseModel):
     )
 
 
+def test_preserve_secrets_in_model_payload_with_secret_dict() -> None:
+    """Restore masked dictionary values without replacing an explicit new secret."""
+    current = _DictSecretSettings()
+    incoming = current.model_dump(mode="json")
+    incoming["secrets"]["token"] = "replacement-token"
+
+    preserved = preserve_secrets_in_model_payload(
+        _DictSecretSettings, current, incoming
+    )
+
+    assert preserved == {
+        "secrets": {
+            "api_key": current.secrets["api_key"].get_secret_value(),
+            "token": "replacement-token",
+        }
+    }
+    assert incoming["secrets"]["api_key"] == SECRET_STR_MASK
+
+
+def test_preserve_secrets_in_model_payload_with_secret_list() -> None:
+    """Restore masked list elements by position and keep explicitly replaced secrets."""
+
+    class _ListOfSecretsSettings(BaseModel):
+        tokens: list[SecretStr]
+
+    current = _ListOfSecretsSettings(
+        tokens=[SecretStr("keep-first"), SecretStr("replace-second")]
+    )
+    incoming = current.model_dump(mode="json")
+    incoming["tokens"][1] = "new-second"
+
+    preserved = preserve_secrets_in_model_payload(
+        _ListOfSecretsSettings, current, incoming
+    )
+
+    assert preserved == {"tokens": ["keep-first", "new-second"]}
+    assert incoming["tokens"][0] == SECRET_STR_MASK
+
+
+@pytest.mark.parametrize(
+    "order", [(2, 0, 1), (0, 2, 1)], ids=["last-first", "first-then-last"]
+)
+def test_preserve_secrets_in_model_payload_matches_reordered_models(
+    order: tuple[int, ...],
+) -> None:
+    """Pair homogeneous model items by their public values rather than PATCH position."""
+
+    class _ListSecretSettings(BaseModel):
+        items: list[_SecretLeafModel]
+
+    current = _ListSecretSettings(
+        items=[
+            _SecretLeafModel(api_key=SecretStr("secret-a"), label="a"),
+            _SecretLeafModel(api_key=SecretStr("secret-b"), label="b"),
+            _SecretLeafModel(api_key=SecretStr("secret-c"), label="c"),
+        ]
+    )
+    incoming = {
+        "items": [current.items[index].model_dump(mode="json") for index in order]
+    }
+
+    preserved = preserve_secrets_in_model_payload(
+        _ListSecretSettings, current, incoming
+    )
+
+    assert preserved == {
+        "items": [
+            {
+                "api_key": current.items[index].api_key.get_secret_value(),
+                "label": current.items[index].label,
+            }
+            for index in order
+        ]
+    }
+
+
+@pytest.mark.parametrize(
+    ("order", "expected_secrets"),
+    [
+        ((1,), ["secret-b", "secret-a", "secret-c"]),
+        ((2, 0), ["secret-c", "secret-a", "secret-b"]),
+    ],
+    ids=["field-overlap-then-position", "sole-unused-item"],
+)
+def test_preserve_secrets_in_model_payload_with_unrecognized_discriminator(
+    order: tuple[int, ...], expected_secrets: list[str]
+) -> None:
+    """Restore masks through fallback matching without reusing a stored item."""
+
+    class _ProviderSecretLeaf(_SecretLeafModel):
+        provider: str = "external"
+
+    class _ListSecretSettings(BaseModel):
+        items: list[_ProviderSecretLeaf]
+
+    current = _ListSecretSettings(
+        items=[
+            _ProviderSecretLeaf(api_key=SecretStr("secret-a"), label="a"),
+            _ProviderSecretLeaf(api_key=SecretStr("secret-b"), label="b"),
+            _ProviderSecretLeaf(api_key=SecretStr("secret-c"), label="c"),
+        ]
+    )
+    incoming_items = [
+        current.items[index].model_dump(mode="json", exclude={"provider"})
+        for index in order
+    ]
+    # This discriminator matches no class name; omit labels to force fallback pairing.
+    fallback = current.items[0].model_dump(mode="json", exclude={"label"})
+    incoming_items.extend(fallback.copy() for _ in range(4 - len(incoming_items)))
+    incoming = {"items": incoming_items}
+
+    preserved = preserve_secrets_in_model_payload(
+        _ListSecretSettings, current, incoming
+    )
+
+    assert preserved == {
+        "items": [
+            {**item, "api_key": secret}
+            for item, secret in zip(
+                incoming_items, [*expected_secrets, SECRET_STR_MASK], strict=True
+            )
+        ]
+    }
+
+
 def test_preserve_patch_secret_value_for_top_level_field() -> None:
     """Assert a masked top-level SecretStr PATCH restores the stored secret."""
     field = _TopLevelSecretSettings.model_fields["TOKEN"]

tests/app/core/settings_override/test_registry_nested.py

@@ -21,7 +21,8 @@
 from typing import ClassVar
 
 import pytest
-from pydantic import BaseModel, SecretStr, ValidationError
+from pydantic import BaseModel, Field, SecretStr, ValidationError
+from sqlmodel.ext.asyncio.session import AsyncSession
 
 from app.core.middleware.security_headers import SecurityHeadersOptions
 from app.core.settings_override.api.routes import _settings_response_from_field
@@ -40,6 +41,7 @@
     nested_overridable_field_names,
     NESTED_VALUE_MISSING,
     not_overridable_field,
+    override_provenance_for_rows,
     ReloadClassification,
     rendered_leaf_keys,
     resolve_nested_field,
@@ -48,6 +50,10 @@
 )
 from app.sep.config import CookieOptions, SEPSettings
 from app.tasks.config import TasksSettings
+from tests.app.core.settings_override.conftest import (
+    insert_override_row,
+    TASKS_SETTINGS_TOKEN,
+)
 
 
 class _Leaf(BaseModel):
@@ -115,6 +121,26 @@ def test_resolve_field_in_model_missing_segment() -> None:
     assert _resolve_field_in_model(CookieOptions, "NOPE") is None
 
 
+@pytest.mark.parametrize("segment", ["External", "Incoming", "Outgoing"])
+def test_resolve_field_in_model_alias_only_match(segment: str) -> None:
+    """Resolve each alias kind independently of the canonical attribute name."""
+
+    class _AliasedModel(BaseModel):
+        value: int = Field(
+            default=1,
+            alias="external",
+            validation_alias="incoming",
+            serialization_alias="outgoing",
+        )
+
+    resolved = _resolve_field_in_model(_AliasedModel, segment)
+
+    assert resolved is not None
+    canonical, field = resolved
+    assert canonical == "value"
+    assert field is _AliasedModel.model_fields["value"]
+
+
 def test_resolve_nested_field_single_level() -> None:
     """A single-level key resolves to a one-segment chain and its leaf field."""
     resolved = resolve_nested_field(SEPSettings, "SESSION_REFRESH__MAX_AGE")
@@ -327,6 +353,33 @@ def test_resolve_nested_field_metadata_reflects_chain_not_overridable() -> None:
     assert locked_leaf.reload is ReloadClassification.NOT_OVERRIDABLE
 
 
+def test_resolve_nested_field_metadata_unknown_leaf_returns_none() -> None:
+    """Return no metadata for an unresolvable nested key."""
+    assert resolve_nested_field_metadata(SEPSettings, "SESSION_REFRESH__BOGUS") is None
+
+
+@pytest.mark.asyncio
+async def test_override_provenance_for_nested_row_includes_all_prefixes(
+    session: AsyncSession,
+) -> None:
+    """Report the stored key and every canonical ancestor of a nested override."""
+    row = await insert_override_row(
+        session,
+        setting_class=TASKS_SETTINGS_TOKEN,
+        key="security_headers__STRICT_TRANSPORT_SECURITY__MAX_AGE",
+        value=3600,
+    )
+
+    provenance = override_provenance_for_rows(TasksSettings, [row])
+
+    assert set(provenance) == {
+        row.key,
+        "SECURITY_HEADERS",
+        "SECURITY_HEADERS__strict_transport_security",
+        "SECURITY_HEADERS__strict_transport_security__max_age",
+    }
+
+
 class _SecretLeafModel(BaseModel):
     """Represent a submodel with a required ``SecretStr`` leaf for resolver tests."""
 
@@ -358,6 +411,45 @@ class _OptionalIntermediateParent(BaseModel):
     NESTED: _OptionalIntermediate = nested_overridable_field(_OptionalIntermediate())
 
 
+def test_resolve_nested_value_unknown_leaf_raises_keyerror() -> None:
+    """Reject a key that cannot resolve to a nested field."""
+    proxy = OverridableSettingsProxy(
+        _OptionalIntermediateParent, setting_class=SEPSettings.__name__
+    )
+
+    with pytest.raises(KeyError, match="NESTED__BOGUS"):
+        resolve_nested_value(
+            settings_cls=_OptionalIntermediateParent,
+            proxy=proxy,
+            key="NESTED__BOGUS",
+        )
+
+
+@pytest.mark.parametrize(
+    ("inner_key", "leaf_key"),
+    [("INNER", "DEEP"), ("inner", "deep")],
+    ids=["exact-mapping-keys", "case-insensitive-mapping-keys"],
+)
+def test_resolve_nested_value_continues_through_mapping(
+    inner_key: str, leaf_key: str
+) -> None:
+    """Traverse a mapping intermediate and read its child using canonical names."""
+    proxy = OverridableSettingsProxy(
+        _OptionalIntermediateParent, setting_class=SEPSettings.__name__
+    )
+    expected_value = 42
+    proxy._set_snapshot({"NESTED": {inner_key: {leaf_key: expected_value}}})
+
+    field, value = resolve_nested_value(
+        settings_cls=_OptionalIntermediateParent,
+        proxy=proxy,
+        key="NESTED__INNER__DEEP",
+    )
+
+    assert value == expected_value
+    assert field is _OptionalInner.model_fields["DEEP"]
+
+
 def test_resolve_nested_value_missing_mapping_segment_returns_sentinel() -> None:
     """Return :data:`NESTED_VALUE_MISSING` for a dict snapshot missing a segment."""
     proxy = OverridableSettingsProxy(