diff --git a/plan/designs/request_path_metering_and_cost_export_design.md b/plan/designs/request_path_metering_and_cost_export_design.md index 5da3660e..4f8792c1 100644 --- a/plan/designs/request_path_metering_and_cost_export_design.md +++ b/plan/designs/request_path_metering_and_cost_export_design.md @@ -644,9 +644,26 @@ remember logout [--token-host URL] There is **no** derivation of a token host from `--api-url`. The engine must not encode a commercial control plane’s `/dp/v1` layout. -`--api-url` on login is the query API stored in the file (default: -`REMEMBERSTACK_API_URL` or `http://127.0.0.1:8000`). It is not the -device-grant host. +`--api-url` on login is an explicit query-API override stored in the file. If +it is omitted, login derives `https://{data_plane_hostname}` from a live +hostname advertised by the token host. It does not fall back to +`REMEMBERSTACK_API_URL` or localhost: doing so could bind a newly minted +deployment credential to an unrelated endpoint. A self-hosted or local token +host that does not advertise a hostname therefore requires the flag. The query +API is not the device-grant host. + +Without the explicit override, the hostname must be present, structurally +valid, and advertised as live. Concretely, it is one printable ASCII host or +`host:port`, with no scheme, path, query, fragment, userinfo, or whitespace; a +port is 1..65535, and the host is either an IP literal or nonempty DNS labels +of at most 63 characters and 253 characters in total. Underscore labels and +one trailing DNS root dot are accepted; an empty label, including one left by +a second trailing dot, is not. A missing hostname asks for `--api-url`; an +invalid hostname or a present hostname whose live flag is false or null exits +nonzero and prints the hostname and reason. These checks occur after the token +is minted, so every refusal withdraws the new credential (or keeps its secret +in the pending-revocation journal when withdrawal cannot be confirmed) and +does not write or replace `credentials.json`. `logout` uses `--token-host`, else `REMEMBERSTACK_TOKEN_HOST`, else the file’s `token_host`. It does not take `--api-url`. @@ -712,10 +729,18 @@ Success **200**: "org_id": "", "deployment_id": "", "label": "", - "token_prefix": "" + "token_prefix": "", + "data_plane_hostname": "", + "data_plane_hostname_live": "" } ``` +The two data-plane fields let login bind the new credential to its deployment. +Older or self-hosted token services may omit them; that is the missing-hostname +case in §6.1. A null live flag is treated as false. The response model ignores +additional fields so the separately deployed token service can evolve without +breaking older clients. + TTL: if authorize’s `expires_in` elapses before 200, stop. Do not keep polling a dead grant. @@ -801,6 +826,9 @@ read if the platform reports a world-readable mode. | Export cursor malformed | 422, no receipts | | Empty page inside horizon | 200 heartbeat | | Login without `--token-host` / env | Exit 2; no derive-from-api-url | +| Login without `--api-url` or an advertised hostname | Exit 1; ask for `--api-url`; withdraw the mint; keep any existing file | +| Login with an invalid advertised hostname | Exit 1; print the hostname; ask for `--api-url`; withdraw the mint; keep any existing file | +| Login with a hostname that is not live | Exit 1; print the hostname; withdraw the mint; keep any existing file | | Logout revoke 5xx | Keep file; exit 1 | | Credential file world-readable | Refuse to read | diff --git a/src/rememberstack/surfaces/cli.py b/src/rememberstack/surfaces/cli.py index 3c503841..b893ea44 100644 --- a/src/rememberstack/surfaces/cli.py +++ b/src/rememberstack/surfaces/cli.py @@ -654,8 +654,10 @@ def _login_locked(args: argparse.Namespace) -> int: from rememberstack.surfaces.device_login import DeviceGrantError from rememberstack.surfaces.device_login import poll_device_token - env = CliClientEnv.model_validate({}) - api_url = args.api_url or env.api_url or "http://127.0.0.1:8000" + # Login binds a newly minted deployment credential. Only the explicit flag + # may override that deployment's advertised host; a process-wide API URL + # can legitimately point at some other deployment. + api_url = args.api_url try: token_host = _resolved_token_host(explicit=args.token_host) except ValueError as error: @@ -762,9 +764,16 @@ def orphaned(payload: object) -> None: # success, take it back out of the journal — it is the current # credential now, not one awaiting revocation. try: - credential = credential_from_token( - token=token, api_url=api_url, token_host=token_host - ) + try: + credential = credential_from_token( + token=token, api_url=api_url, token_host=token_host + ) + except DeviceGrantError: + # The poll already journalled the minted bearer. Retire it + # now when possible; if the host cannot confirm that, the + # journal keeps the only secret needed for a later retry. + _retry_pending_revocation() + raise if existing is not None: # Written before the file is overwritten, because # overwriting it destroys the only copy of the @@ -811,9 +820,10 @@ def orphaned(payload: object) -> None: ) ) except BaseException: - # The journal entry stays: whatever went wrong, the credential - # exists at the token host and the next login or logout will - # retire it. Nothing here has to succeed for that to hold. + # Unless the hostname-refusal path above already retired the + # mint, its journal entry stays: the credential exists at the + # token host and the next login or logout will retire it. + # Nothing here has to succeed for that to hold. raise except KeyboardInterrupt: return 130 @@ -844,6 +854,14 @@ def orphaned(payload: object) -> None: print(f"api_url: {credential.api_url}") if credential.expires_at is not None: print(f"expires_at: {credential.expires_at.isoformat()}") + env_api_url = CliClientEnv.model_validate({}).api_url + if env_api_url and env_api_url != credential.api_url: + print( + f"warning: REMEMBERSTACK_API_URL={env_api_url} overrides the " + f"stored api_url {credential.api_url} for other commands; " + "unset it to use this deployment", + file=sys.stderr, + ) return 0 diff --git a/src/rememberstack/surfaces/device_login.py b/src/rememberstack/surfaces/device_login.py index c82bc977..0a96b9ae 100644 --- a/src/rememberstack/surfaces/device_login.py +++ b/src/rememberstack/surfaces/device_login.py @@ -9,12 +9,15 @@ from collections.abc import Callable from collections.abc import Mapping from datetime import datetime +from ipaddress import ip_address import time +from typing import Annotated from typing import Literal from uuid import UUID import httpx from pydantic import BaseModel +from pydantic import BeforeValidator from pydantic import ConfigDict from pydantic import Field from pydantic import SecretStr @@ -67,9 +70,9 @@ class DeviceTokenSuccess(BaseModel): server may add, and the client must carry on. Fields this client actually needs are declared and validated; anything else is the server's business. - Known-but-unused fields are declared explicitly rather than swallowed, so a - reader can see what the server sends and a future change to use one does not - have to rediscover it. + The advertised data-plane hostname is declared explicitly because login uses + it to configure managed deployments without a separate ``--api-url``. Other + additive fields remain safe to ignore until the client needs them. """ model_config = ConfigDict(extra="ignore", frozen=True, hide_input_in_errors=True) @@ -85,7 +88,9 @@ class DeviceTokenSuccess(BaseModel): #: Advertised by the control plane; the CLI stores it so a caller does not #: have to be told the host separately. data_plane_hostname: str | None = None - data_plane_hostname_live: bool = False + data_plane_hostname_live: Annotated[ + bool, BeforeValidator(lambda value: False if value is None else value) + ] = False #: When the credential stops working (D60). Absent for the unexpiring #: tokens minted before that decision, which is why it is optional rather #: than required — a client that demanded it would refuse today's tokens. @@ -283,9 +288,27 @@ def revoke_self(*, client: httpx.Client, access_token: str) -> int: def credential_from_token( - *, token: DeviceTokenSuccess, api_url: str, token_host: str + *, token: DeviceTokenSuccess, api_url: str | None, token_host: str ) -> CredentialFile: - """Build the v1 credential document from a successful poll.""" + """Build the v1 credential document, deriving its managed API URL.""" + api_url = (api_url or "").strip() + if not api_url: + hostname = (token.data_plane_hostname or "").strip() + if not hostname: + raise DeviceGrantError( + "token host did not advertise a data-plane hostname; pass --api-url" + ) + if not _valid_data_plane_hostname(hostname): + raise DeviceGrantError( + "token host advertised an invalid data-plane hostname: " + f"{hostname!r}; pass --api-url to override" + ) + if not token.data_plane_hostname_live: + raise DeviceGrantError( + f"deployment {hostname!r} is not live yet; " + "run `remember login` again when it is" + ) + api_url = f"https://{hostname}" return CredentialFile( version=1, api_url=api_url, @@ -301,6 +324,54 @@ def credential_from_token( ) +def _valid_data_plane_hostname(hostname: str) -> bool: + """Accept one printable URL authority with a real DNS or IP host.""" + if ( + not hostname.isascii() + or not hostname.isprintable() + or any(character.isspace() for character in hostname) + ): + return False + try: + url = httpx.URL(f"https://{hostname}") + host = url.host + port = url.port + userinfo = url.userinfo + path = url.path + query = url.query + fragment = url.fragment + except (httpx.InvalidURL, UnicodeError): + return False + if ( + not host + or userinfo + or path != "/" + or query + or fragment + or "@" in hostname + or "/" in hostname + or hostname.endswith(":") + or (port is not None and not 1 <= port <= 65535) + ): + return False + try: + ip_address(host) + except ValueError: + if len(host) > 253: + return False + stem = host[:-1] if host.endswith(".") else host + labels = stem.split(".") + return all( + label + and len(label) <= 63 + and (label[0].isalnum() or label[0] == "_") + and (label[-1].isalnum() or label[-1] == "_") + and all(character.isalnum() or character in "-_" for character in label) + for label in labels + ) + return True + + def request_same_origin( *, client: httpx.Client, diff --git a/src/tests/surfaces/test_device_login_contract.py b/src/tests/surfaces/test_device_login_contract.py index 065a7cd5..8b67c20c 100644 --- a/src/tests/surfaces/test_device_login_contract.py +++ b/src/tests/surfaces/test_device_login_contract.py @@ -49,6 +49,16 @@ def test_todays_control_plane_response_parses() -> None: assert parsed.data_plane_hostname_live is True +def test_an_unprovisioned_deployment_may_report_null_status() -> None: + """A nullable hostname may naturally arrive with a nullable live flag.""" + parsed = DeviceTokenSuccess.model_validate( + _control_plane_body(data_plane_hostname=None, data_plane_hostname_live=None) + ) + + assert parsed.data_plane_hostname is None + assert parsed.data_plane_hostname_live is False + + def test_an_expiry_the_server_starts_sending_parses() -> None: """D60 adds `expires_at`; a client must absorb it rather than break.""" expires_at = datetime.now(timezone.utc) + timedelta(days=365) diff --git a/src/tests/surfaces/test_login.py b/src/tests/surfaces/test_login.py index 8736ab1e..dc996907 100644 --- a/src/tests/surfaces/test_login.py +++ b/src/tests/surfaces/test_login.py @@ -449,12 +449,19 @@ def _token_body(**overrides: object) -> dict[str, object]: return body -def _grant_handler(*, token_body: dict[str, object], calls: list[str]) -> "object": +def _grant_handler( + *, + token_body: dict[str, object], + calls: list[str], + revoked_authorizations: list[str] | None = None, +) -> "object": """Serve authorize/token/revoke, recording the order they are called in.""" def handler(request: httpx.Request) -> httpx.Response: if request.method == "DELETE": calls.append("revoke") + if revoked_authorizations is not None: + revoked_authorizations.append(request.headers["authorization"]) return httpx.Response(204) if request.url.path == "/v1/device/authorize": calls.append("authorize") @@ -488,6 +495,173 @@ def factory(*args: object, **kwargs: object) -> httpx.Client: monkeypatch.setattr(httpx, "Client", factory) +def test_login_derives_the_api_url_from_a_live_hostname( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A managed login needs only the token host once its deployment is live.""" + _isolate_config(monkeypatch, tmp_path) + monkeypatch.setenv("REMEMBERSTACK_API_URL", "https://stale.example.test") + calls: list[str] = [] + hostname = f"{_DEPLOYMENT_ID}.dp.remember.dev" + _mock_client( + monkeypatch, + _grant_handler( + token_body=_token_body( + data_plane_hostname=hostname, data_plane_hostname_live=True + ), + calls=calls, + ), + ) + + assert cli_main(["login", "--token-host", _TOKEN_HOST]) == 0 + stored = load_credentials() + assert stored is not None + assert stored.api_url == f"https://{hostname}" + assert calls == ["authorize", "token"] + assert ( + "warning: REMEMBERSTACK_API_URL=https://stale.example.test overrides " + f"the stored api_url https://{hostname} for other commands" + in capsys.readouterr().err + ) + + +def test_login_api_url_override_wins_over_an_unready_hostname( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Self-host and local users can explicitly choose their API endpoint.""" + _isolate_config(monkeypatch, tmp_path) + calls: list[str] = [] + _mock_client( + monkeypatch, + _grant_handler( + token_body=_token_body( + data_plane_hostname="not-live.dp.remember.dev", + data_plane_hostname_live=False, + ), + calls=calls, + ), + ) + + assert cli_main(["login", "--token-host", _TOKEN_HOST, "--api-url", _API]) == 0 + stored = load_credentials() + assert stored is not None + assert stored.api_url == _API + assert calls == ["authorize", "token"] + + +def test_login_normalizes_a_hostname_and_treats_a_blank_override_as_absent( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Whitespace cannot turn a valid managed endpoint into a broken URL.""" + _isolate_config(monkeypatch, tmp_path) + calls: list[str] = [] + hostname = f"svc_1.{_DEPLOYMENT_ID}.dp.remember.dev." + _mock_client( + monkeypatch, + _grant_handler( + token_body=_token_body( + data_plane_hostname=f" {hostname} ", data_plane_hostname_live=True + ), + calls=calls, + ), + ) + + assert cli_main(["login", "--token-host", _TOKEN_HOST, "--api-url", " "]) == 0 + stored = load_credentials() + assert stored is not None + assert stored.api_url == f"https://{hostname}" + assert calls == ["authorize", "token"] + + +def test_login_refuses_a_hostname_that_is_not_live( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """Do not persist a managed endpoint before the control plane says it is live.""" + _isolate_config(monkeypatch, tmp_path) + predecessor = _stored(token_prefix="old-prefix") + write_credentials(credential=predecessor) + calls: list[str] = [] + revoked_authorizations: list[str] = [] + hostname = f"{_DEPLOYMENT_ID}.dp.remember.dev" + _mock_client( + monkeypatch, + _grant_handler( + token_body=_token_body( + data_plane_hostname=hostname, data_plane_hostname_live=False + ), + calls=calls, + revoked_authorizations=revoked_authorizations, + ), + ) + + assert cli_main(["login", "--token-host", _TOKEN_HOST]) == 1 + captured = capsys.readouterr() + assert hostname in captured.err + assert f"error: deployment {hostname!r} is not live yet" in captured.err + assert "run `remember login` again when it is" in captured.err + stored = load_credentials() + assert stored is not None + assert stored.token_id == predecessor.token_id + assert stored.access_token == predecessor.access_token + assert calls == ["authorize", "token", "revoke"] + assert revoked_authorizations == [f"Bearer {_ACCESS}-new"] + + +@pytest.mark.parametrize( + "hostname", + [ + "https://deployment.example.test/path", + "@deployment.example.test", + "deployment.example.test:8443/", + ":8443", + "[::1", + "evil\x1b[2m.example", + "a\x00b", + "xn--0.com", + "deployment.example.test..", + ], +) +def test_login_refuses_an_invalid_advertised_hostname( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + hostname: str, +) -> None: + """A token response cannot persist a scheme or path as though it were a host.""" + _isolate_config(monkeypatch, tmp_path) + calls: list[str] = [] + _mock_client( + monkeypatch, + _grant_handler( + token_body=_token_body( + data_plane_hostname=hostname, data_plane_hostname_live=True + ), + calls=calls, + ), + ) + + assert cli_main(["login", "--token-host", _TOKEN_HOST]) == 1 + captured = capsys.readouterr() + assert repr(hostname) in captured.err + assert "pass --api-url to override" in captured.err + assert load_credentials() is None + assert calls == ["authorize", "token", "revoke"] + + +def test_login_without_an_advertised_hostname_asks_for_api_url( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """Older and self-hosted token services still need an explicit API URL.""" + _isolate_config(monkeypatch, tmp_path) + calls: list[str] = [] + _mock_client(monkeypatch, _grant_handler(token_body=_token_body(), calls=calls)) + + assert cli_main(["login", "--token-host", _TOKEN_HOST]) == 1 + assert "--api-url" in capsys.readouterr().err + assert load_credentials() is None + assert calls == ["authorize", "token", "revoke"] + + def test_login_revokes_the_predecessor_only_after_minting( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/website/src/app/docs/configuration/page.mdx b/website/src/app/docs/configuration/page.mdx index 3ef6ceb0..8f6a9330 100644 --- a/website/src/app/docs/configuration/page.mdx +++ b/website/src/app/docs/configuration/page.mdx @@ -51,7 +51,7 @@ Authoritative template: repository [`.env.example`](https://github.com/writeitai | `REMEMBERSTACK_SELFHOST_REQUIRE_API_AUTH` | When `true`, the API process refuses to start unless it has a perimeter — `API_BEARER_BIND`, `API_SIGNING_KEYS`, or both. Default `false` (open quickstart) | | `REMEMBERSTACK_SELFHOST_TRUSTED_PRINCIPAL_SOURCE` | Whether `X-Ingest-Principal-*` attribution is believed. Default `false`; enable only behind a network perimeter that authenticates the asserted actor. With API auth configured, the presenting credential also needs full `write` authority | | `REMEMBERSTACK_SELFHOST_BROWSER_ORIGINS` | Optional comma-separated `https://` origins allowed to call this deployment from a browser. Empty by default, which advertises no CORS at all. Each entry must be an exact scheme-and-host origin — no wildcard, no path, no `http://` | -| `REMEMBERSTACK_API_URL` | Client target; defaults to `http://127.0.0.1:8000` — **must match** if you change the port | +| `REMEMBERSTACK_API_URL` | SDK and memory-command target; defaults to `http://127.0.0.1:8000` — **must match** if you change the port. `remember login` instead uses its explicit `--api-url` or the live hostname advertised by the token host | | `REMEMBERSTACK_TOKEN_HOST` | Device-grant host for `remember login` (required unless `--token-host` is passed). Never derived from the query API URL | | `REMEMBERSTACK_CONFIG_DIR` | Optional override for the CLI credential directory. Holds `credentials.json` and, while a replaced credential is still awaiting revocation, `pending-revocation.json` — both `0600` inside a `0700` directory | | `REMEMBERSTACK_COST_EXPORT_BIND` | Optional second listen address for HTTP cost export (`127.0.0.1:8001`, `[::1]:8001`, or `unix:/path`). Unset = no HTTP export | diff --git a/website/src/app/docs/project-status/page.mdx b/website/src/app/docs/project-status/page.mdx index 5cd887a1..bc0fe3e3 100644 --- a/website/src/app/docs/project-status/page.mdx +++ b/website/src/app/docs/project-status/page.mdx @@ -156,6 +156,15 @@ Current public release: [`v0.15.0`](https://github.com/writeitai/remember-stack/ The resolver generation moves to `resolver-2026.08g`, so evaluation curves measured under `08f` are not comparable and a fresh run is required. +## What landed after v0.15.0 (on `main`) + +**Managed login uses the deployment hostname it receives.** When the token +host advertises a live data-plane hostname, `remember login` derives the query +API URL, so managed users need only `--token-host`. An explicit `--api-url` +still wins for self-hosted and local deployments. A missing hostname asks for +that flag, while a hostname that is not live prints the name and asks the user +to retry; neither refusal replaces an existing credential. + ## What landed after v0.14.0 (in v0.15.0) **A deployment can tell you what it holds.** `GET /documents` lists document diff --git a/website/src/app/docs/reference/cli/page.mdx b/website/src/app/docs/reference/cli/page.mdx index 3b027b92..70394edf 100644 --- a/website/src/app/docs/reference/cli/page.mdx +++ b/website/src/app/docs/reference/cli/page.mdx @@ -40,13 +40,32 @@ remember --version Device-grant login lives in the base wheel. `--token-host` (or `REMEMBERSTACK_TOKEN_HOST`) is required on login and is **never** derived from `--api-url`. The token host is the service that mints deployment tokens; it is -not the query API. +not the query API. For a managed deployment whose advertised hostname is live, +the token response supplies the API URL, so only the token host is needed: + +```bash +remember login --token-host https://token-host.example +``` + +For a self-hosted or local deployment, pass its API URL explicitly. This +override wins even if the token host advertises a different or not-yet-live +hostname. `REMEMBERSTACK_API_URL` remains the default for other client +commands, but it is not a login override: login uses only an explicit +`--api-url` or the hostname bound to the newly minted credential. When that +environment variable points somewhere else, a successful login warns that it +will override the stored deployment URL for subsequent commands. ```bash remember login --token-host https://token-host.example --api-url http://127.0.0.1:8000 remember logout ``` +Without `--api-url`, login refuses an advertised hostname that is not live, +prints that hostname, and asks you to run `remember login` again when it is. If +the token host advertises no hostname, login asks for `--api-url`. Neither +refusal writes a credential file. An advertised value that is not a plain host +or `host:port` is also refused; pass `--api-url` to override it. + `login` prints the user code and both verification URLs, then polls `POST {token_host}/v1/device/token`. It never prints `device_code` or `access_token`. The credential file is