Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/tools/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
26 changes: 26 additions & 0 deletions src/osw/service/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down
73 changes: 73 additions & 0 deletions tests/test_service_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import os
import sys
from pathlib import Path

import pytest
import yaml
Expand Down Expand Up @@ -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):
Expand Down
Loading