fix(config): validate domain and state_dir (#195)

* fix(config): reject a hostless domain and a relative state_dir

- domain accepts a bare host or a full URL, so the check is on what
  _derive_domain yields rather than on the form of the value
- a hostless domain used to fail later in OswExpress.validate_domain,
  quoting a regex instead of naming OSW_DOMAIN
- state_dir "~/osw" created a directory literally named "~", because
  Ledger builds its path with Path() and never expands it
- a relative state_dir resolves against a working directory the MCP
  client chooses, so the ledger landed in an unpredictable place
- last open item from #143

* test(config): cover drive-relative state_dir and an unknown home

- reject "\osw-state" and "C:osw-state", non-absolute on both platforms
- assert the expanduser RuntimeError becomes a named config error
- state in the docs that a domain without a host is rejected

Author: LukasGold

docs/tools/configuration.md

@@ -100,14 +100,14 @@ set wins:
 
 | Canonical | Also accepted | Meaning |
 | --- | --- | --- |
-| `OSW_DOMAIN` | `OSL_DOMAIN` | Instance to connect to |
+| `OSW_DOMAIN` | `OSL_DOMAIN` | Instance to connect to. A bare host (`wiki.example.org`) or a full URL (`https://wiki.example.org/w/`); the host is taken from either, and a value no host can be read from (`https://`, `/w/index.php`) is rejected at startup |
 | `OSW_USERNAME` | `OSL_USERNAME` | Login user |
 | `OSW_PASSWORD` | `OSL_PASSWORD` | Login password |
 | `OSW_CRED_FILEPATH` | `OSW_MCP_CRED_FILEPATH`, `OSL_CRED_FILEPATH` | YAML credential file, keyed by iri (falls back to `accounts.pwd.yaml` in the working directory, CLI only) |
 | `OSW_ENV_FILE` | `OSW_MCP_ENV_FILE` | `.env` file to load |
 | `OSW_READ_ONLY` | `OSW_MCP_READ_ONLY` | `true` refuses every write |
 | `OSW_SPARQL_ENDPOINT` | | Endpoint for `sparql` queries |
-| `OSW_STATE_DIR` | `OSW_MCP_STATE_DIR` | Where the provenance ledger is kept |
+| `OSW_STATE_DIR` | `OSW_MCP_STATE_DIR` | Where the provenance ledger is kept. Must be an absolute path; a leading `~` is expanded |
 | `OSW_MAX_RESULTS` | `OSW_MCP_MAX_RESULTS` | Default result cap (100) |
 | `OSW_MAX_CHARS` | `OSW_MCP_MAX_CHARS` | Result size cap in characters (100000) |
 | `OSW_VERBOSE` | `OSW_MCP_VERBOSE` | `true` prints the configuration source report |

src/osw/service/config.py

@@ -117,6 +117,16 @@ def _validate_domain(cls, value: Optional[str]) -> Optional[str]:
             raise ValueError("must not contain whitespace")
         if any(ord(char) < 32 for char in value):
             raise ValueError("must not contain control characters")
+        # A bare host and a full URL are both legal here, so the check is on
+        # what _derive_domain makes of the value, not on its form.
+        # OswExpress.validate_domain rejects a hostless value too, but only on
+        # the first connection, and its message quotes a regex rather than
+        # naming the variable the operator has to correct.
+        if not _derive_domain(value):
+            raise ValueError(
+                "must contain a host name (e.g. 'wiki.example.org' or "
+                "'https://wiki.example.org/w/')"
+            )
         return value
 
     @field_validator("sparql_endpoint")
@@ -138,6 +148,22 @@ def _validate_state_dir(cls, value: Optional[str]) -> Optional[str]:
             return value
         if not value.strip():
             raise ValueError("must not be empty or whitespace-only")
+        # The only validator here that rewrites its value. Ledger builds its
+        # file as Path(state_dir) / ... and never expands a leading ~, so
+        # "~/osw" used to create a directory literally named "~".
+        if value.startswith("~"):
+            try:
+                value = str(Path(value).expanduser())
+            except RuntimeError as exc:
+                raise ValueError(
+                    f"starts with '~' but the home directory cannot be "
+                    f"determined ({exc})"
+                ) from exc
+        if not Path(value).is_absolute():
+            raise ValueError(
+                "must be an absolute path: a relative one resolves against the "
+                "working directory, which for osw-mcp is chosen by the client"
+            )
         return value
 
     @field_validator("cred_filepath")

tests/test_service_config.py

@@ -2,6 +2,7 @@
 
 import os
 import sys
+from pathlib import Path
 
 import pytest
 import yaml
@@ -1055,6 +1056,78 @@ def test_domain_as_full_url_accepted():
     assert settings.domain == "https://wiki.example.org/w/"
 
 
+@pytest.mark.parametrize("value", ["https://", "/w/index.php", "//", "https:///w/"])
+def test_domain_without_a_host_rejected(value):
+    """A value _derive_domain cannot reduce to a host is unusable.
+
+    Caught here rather than in OswExpress.validate_domain, which only runs on
+    the first connection and reports a regex rather than the variable name.
+    """
+    with pytest.raises(ValidationError) as exc:
+        Settings(domain=value)
+    assert "host" in str(exc.value)
+
+
+def test_state_dir_expands_a_leading_tilde():
+    """Path(state_dir) never expands it, so '~/osw' made a directory named '~'.
+
+    Ledger builds its file as Path(state_dir) / ... with no expanduser call
+    (src/osw/service/ledger.py:70), so the expansion has to happen here.
+    """
+    settings = Settings(domain="wiki.example.org", state_dir="~/osw-state")
+    assert settings.state_dir == str(Path.home() / "osw-state")
+
+
+@pytest.mark.parametrize(
+    "value",
+    [
+        "osw-state",
+        "./osw-state",
+        "../osw-state",
+        # Drive-relative Windows forms: rooted without a drive, and a drive
+        # without a root. Both resolve against process state (the current drive,
+        # and that drive's working directory), so both are as unpredictable as a
+        # plain relative path. is_absolute() is False for both on Windows and on
+        # POSIX, so these parameters need no platform marker.
+        "\\osw-state",
+        "C:osw-state",
+    ],
+)
+def test_state_dir_relative_rejected(value):
+    """A relative path resolves against a working directory the user may not own.
+
+    The MCP client chooses the server's working directory, so the ledger would
+    be created in an unpredictable place.
+    """
+    with pytest.raises(ValidationError) as exc:
+        Settings(domain="wiki.example.org", state_dir=value)
+    assert "absolute" in str(exc.value)
+
+
+def test_state_dir_reports_an_undeterminable_home(monkeypatch):
+    """The '~' expansion can fail, and the failure has to name the setting.
+
+    Path.expanduser() raises RuntimeError when no home directory can be found.
+    Uncaught it would surface as a bare RuntimeError with no mention of
+    OSW_STATE_DIR. Reproducing that state differs per platform (Windows reads
+    USERPROFILE, POSIX falls back to the password database), so the raise itself
+    is patched in.
+    """
+
+    def _no_home(self):
+        raise RuntimeError("Could not determine home directory.")
+
+    monkeypatch.setattr(Path, "expanduser", _no_home)
+    with pytest.raises(ValidationError) as exc:
+        Settings(domain="wiki.example.org", state_dir="~/osw-state")
+    assert "home directory" in str(exc.value)
+
+
+def test_state_dir_absolute_is_left_alone(tmp_path):
+    settings = Settings(domain="wiki.example.org", state_dir=str(tmp_path / "state"))
+    assert settings.state_dir == str(tmp_path / "state")
+
+
 def test_settings_is_frozen():
     settings = Settings(domain="wiki.example.org")
     with pytest.raises(ValidationError):