From afefbfe42ef6eb980294e8659b517cc7e35dbd41 Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:10:50 +1000 Subject: [PATCH 1/2] feat(share): keep OAuth identity across ordinary share restarts An OAuth share persisted its client but retired grants and minted a fresh random session subject on every start, forcing reauthorization for an ordinary restart. Add an explicit --oauth-persist-on-restart opt-in that keeps the validated client and its unexpired, unrevoked grants via a session subject derived from the persisted client id. Expiry, revocation, scope and replay rules are unchanged and the default behaviour is identical. --- docs/CLI.md | 2 +- docs/CLI.zh-CN.md | 2 +- scripts/e2e_share_oauth_restart.py | 293 +++++++++++++++++++++++++++++ src/auth/mod.rs | 4 +- src/auth/project_share.rs | 48 +++++ src/project_entry.rs | 4 +- src/project_entry_share.rs | 197 ++++++++++++++++++- 7 files changed, 540 insertions(+), 10 deletions(-) create mode 100755 scripts/e2e_share_oauth_restart.py diff --git a/docs/CLI.md b/docs/CLI.md index 5553e4350..279e6f7da 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -119,7 +119,7 @@ These commands work on the current Git project. `webcodex share --auth query-token` is an explicit temporary-share compatibility mode for MCP clients that cannot configure a Bearer header. It accepts the exact share Project Credential only on `/mcp?token=...`, prints a URL-encoded sensitive MCP URL, and tells the client to use No authentication. The mode is disabled for ordinary Server/runtime requests, does not accept PAT/OAuth/shared-key/Runner credentials through the query, and is rejected with `--tunnel openai`. Treat the complete URL as a credential because URL queries may be retained by clients, proxies, clipboards, or access logs. The default remains `--auth bearer`. -`webcodex share --auth oauth --oauth-redirect-uri ` uses OAuth 2.0 Authorization Code with PKCE S256. The OAuth client ID/secret are persisted in protected project state for that project + callback, while the temporary OAuth grants are valid only for the current `share` run. Restarting `share` therefore invalidates old OAuth grants without changing the project. OAuth access tokens are never accepted on Runner transport. +`webcodex share --auth oauth --oauth-redirect-uri ` uses OAuth 2.0 Authorization Code with PKCE S256. The OAuth client ID/secret are persisted in protected project state for that project + callback, while the temporary OAuth grants are valid only for the current `share` run. Restarting `share` therefore invalidates old OAuth grants without changing the project. OAuth access tokens are never accepted on Runner transport. Add `--oauth-persist-on-restart` to keep the validated OAuth client and its unexpired, unrevoked grants across ordinary restarts of the same share: the session subject is derived from the persisted client ID instead of a fresh random value, so a restarted share does not force reauthorization. Expiration, revocation, scope and token replay rules remain authoritative, a restarted share replays no effects, and the first run with the flag can require one reauthorization because the previous run's grants were issued under a different session subject. Without the flag, behavior is unchanged. Quick Tunnel origins remain temporary. For an operator-managed stable HTTPS origin, use `--tunnel none --public-url https://share.example` and route that origin to the loopback WebCodex Server yourself; `--public-url` advertises the external origin/issuer and does not create a proxy or tunnel. diff --git a/docs/CLI.zh-CN.md b/docs/CLI.zh-CN.md index 6d41ffbb7..dd8dd2643 100644 --- a/docs/CLI.zh-CN.md +++ b/docs/CLI.zh-CN.md @@ -68,7 +68,7 @@ LaunchAgent 与 Windows 用户计划任务需要该用户保持登录。旧服 `webcodex share --auth query-token` 是给无法配置 Bearer header 的 MCP client 使用的显式临时 share 兼容模式。它只在 `/mcp?token=...` 接受当前 share 的精确 Project Credential，输出经过 URL 编码的敏感 MCP URL，并让 client 选择 No authentication。普通 Server/runtime 请求不会因此启用 query auth，PAT/OAuth/shared-key/Runner credential 也不能通过这个 query 路径回退认证；`--tunnel openai` 会拒绝该模式。整条 URL 都必须当作 credential，因为 query 可能被 client、proxy、剪贴板或 access log 留存。默认仍是 `--auth bearer`。 -`webcodex share --auth oauth --oauth-redirect-uri <精确回调地址>` 使用 OAuth 2.0 Authorization Code + PKCE S256。OAuth client ID/secret 会按“项目 + 回调地址”保存在受保护的 project state 中；临时 OAuth grant 只在当前 `share` 运行期间有效。重启 `share` 会让旧 OAuth grant 失效，但不会改变项目。OAuth access token 永远不能用于 Runner transport。 +`webcodex share --auth oauth --oauth-redirect-uri <精确回调地址>` 使用 OAuth 2.0 Authorization Code + PKCE S256。OAuth client ID/secret 会按“项目 + 回调地址”保存在受保护的 project state 中；临时 OAuth grant 只在当前 `share` 运行期间有效。重启 `share` 会让旧 OAuth grant 失效，但不会改变项目。OAuth access token 永远不能用于 Runner transport。加 `--oauth-persist-on-restart` 后，同一个 share 的普通重启会保留已校验的 OAuth client 及其未过期、未撤销的 grant：session subject 由持久化的 client ID 派生，而不是每次重新随机生成，因此重启不必重新授权。过期、撤销、scope 与 token replay 规则仍然有效，重启不会重放任何副作用；首次带该标志运行时可能需要一次重新授权，因为上一次的 grant 属于不同的 session subject。不加该标志时行为完全不变。 Cloudflare Quick Tunnel 的公网 origin 仍然是临时的。如需稳定 HTTPS origin，可使用 `--tunnel none --public-url https://share.example`，并由 operator 自己把该 origin 反向代理/隧道到 loopback WebCodex Server；`--public-url` 只声明外部 origin/issuer，不会创建代理或 tunnel。 diff --git a/scripts/e2e_share_oauth_restart.py b/scripts/e2e_share_oauth_restart.py new file mode 100755 index 000000000..85cc81bae --- /dev/null +++ b/scripts/e2e_share_oauth_restart.py @@ -0,0 +1,293 @@ +#!/usr/bin/env python3 +"""Loopback-only OAuth restart-persistence acceptance with a real share process. + +Build the binaries first: + cargo build --locked --profile dogfood -p webcodex -p webcodex-cli -p webcodex-runner + python3 scripts/e2e_share_oauth_restart.py --bin-dir target/dogfood + +Everything runs on 127.0.0.1: a real "webcodex share --tunnel none --auth oauth" +process owns a real Server and WebSocket Runner, the harness completes the real +OAuth authorization-code + PKCE S256 flow with the share's own client, then +stops and restarts only its own process. No external service, production state, +real user token or TLS verification bypass is used. +""" +from __future__ import annotations + +import argparse +import base64 +import hashlib +import http.client +import json +import os +from pathlib import Path +import re +import secrets +import shutil +import signal +import socket +import subprocess +import sys +import tempfile +import time +import urllib.parse + +MCP_PROTOCOL = "2026-07-28" +SCOPES = "runtime:read session:collaborate project:read project:write job:run" + + +def check(value: object, message: str) -> None: + if not value: + raise AssertionError(message) + + +def reserve_port() -> int: + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +def git(repo: Path, *args: str) -> str: + result = subprocess.run( + ["git", *args], cwd=repo, capture_output=True, text=True, check=True + ) + return result.stdout.strip() + + +def make_git_repo(path: Path) -> None: + path.mkdir(parents=True, exist_ok=True) + git(path, "init", "-b", "main") + git(path, "config", "user.email", "fixture@example.invalid") + git(path, "config", "user.name", "Share OAuth Restart Fixture") + (path / "README.md").write_text("oauth restart fixture\n") + git(path, "add", "-A") + git(path, "commit", "-m", "fixture root commit") + + +class Share: + def __init__(self, bin_dir: Path, root: Path) -> None: + self.bin_dir = bin_dir + self.root = root + self.public_port = reserve_port() + self.origin = f"http://127.0.0.1:{self.public_port}" + self.port: int | None = None + self.process: subprocess.Popen | None = None + self.log_path = root / "share.log" + self.rpc_id = 0 + + def launch(self, project_dir: Path, persist: bool, tag: str) -> None: + log_path = self.root / f"share-{self.public_port}-{tag}.log" + log_path.write_bytes(b"") + self.log_path = log_path + command = [ + str(self.bin_dir / "webcodex"), "share", + "--tunnel", "none", + "--public-url", self.origin, + "--auth", "oauth", + "--oauth-redirect-uri", f"{self.origin}/callback", + "--no-copy-url", + "--state-dir", str(self.root / "state"), + ] + if persist: + command.append("--oauth-persist-on-restart") + log = open(log_path, "ab") + self.process = subprocess.Popen( + command, cwd=project_dir, stdout=log, stderr=log, start_new_session=True + ) + + def discover_server_port(self) -> None: + configs = list((self.root / "state").glob("**/runner.toml")) + check(bool(configs), "generated Runner config not found under state dir") + match = re.search(r'server_url\s*=\s*"http://127\.0\.0\.1:(\d+)"', configs[0].read_text()) + check(bool(match), f"server_url missing from {configs[0]}") + self.port = int(match.group(1)) + + def wait_ready(self, deadline_secs: float = 90.0) -> dict: + deadline = time.monotonic() + deadline_secs + ready_re = re.compile( + r"4\. Client ID: (?P\S+)\n.*?5\. Client secret: (?P\S+)\n" + r".*?Project share credential \(this share only\): (?P\S+)", + re.S, + ) + while time.monotonic() < deadline: + if self.process is not None and self.process.poll() is not None: + raise AssertionError( + f"share exited early with {self.process.returncode}; see {self.log_path}" + ) + match = ready_re.search(self.log_path.read_text(errors="replace")) + if match: + self.discover_server_port() + return { + "client_id": match.group("client"), + "client_secret": match.group("secret"), + "credential": match.group("credential"), + } + time.sleep(0.3) + raise AssertionError(f"share readiness deadline exceeded; see {self.log_path}") + + def stop(self) -> None: + if self.process is None: + return + try: + os.killpg(self.process.pid, signal.SIGTERM) + except ProcessLookupError: + pass + try: + self.process.wait(timeout=20) + except subprocess.TimeoutExpired: + try: + os.killpg(self.process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + self.process.wait(timeout=20) + self.process = None + + def request(self, path: str, method: str = "GET", body: bytes | None = None, + headers: dict | None = None) -> tuple[int, dict, bytes]: + headers = dict(headers or {}) + if body is not None and "Content-Type" not in headers: + headers["Content-Type"] = "application/x-www-form-urlencoded" + check(self.port is not None, "server port not discovered yet") + connection = http.client.HTTPConnection("127.0.0.1", self.port, timeout=30) + try: + connection.request(method, path, body=body, headers=headers) + response = connection.getresponse() + data = response.read(2 * 1024 * 1024 + 1) + check(len(data) <= 2 * 1024 * 1024, "response exceeded bound") + return response.status, {k.lower(): v for k, v in response.getheaders()}, data + finally: + connection.close() + + def authorize(self, client: dict) -> dict: + verifier = secrets.token_urlsafe(48) + challenge = base64.urlsafe_b64encode( + hashlib.sha256(verifier.encode()).digest() + ).decode().rstrip("=") + parameters = { + "response_type": "code", + "client_id": client["client_id"], + "redirect_uri": f"{self.origin}/callback", + "scope": SCOPES, + "state": secrets.token_urlsafe(16), + "resource": self.origin + "/mcp", + "code_challenge": challenge, + "code_challenge_method": "S256", + } + status, _, _ = self.request("/oauth/authorize?" + urllib.parse.urlencode(parameters)) + check(status == 200, "project share authorize form unavailable") + form = dict(parameters) + form["project_credential"] = client["credential"] + status, headers, _ = self.request( + "/oauth/authorize/project", "POST", urllib.parse.urlencode(form).encode(), + headers={"Origin": self.origin}, + ) + check(status == 302, f"project share authorize rejected: {status}") + query = urllib.parse.parse_qs(urllib.parse.urlsplit(headers.get("location", "")).query) + check(query.get("state") == [parameters["state"]] and "code" in query, + "OAuth redirect/code mismatch") + status, _, raw = self.request("/oauth/token", "POST", urllib.parse.urlencode({ + "grant_type": "authorization_code", + "code": query["code"][0], + "client_id": client["client_id"], + "client_secret": client["client_secret"], + "redirect_uri": f"{self.origin}/callback", + "resource": self.origin + "/mcp", + "code_verifier": verifier, + }).encode()) + check(status == 200, f"S256 code exchange denied: {status} {raw[:400]!r}") + tokens = json.loads(raw) + check(isinstance(tokens.get("access_token"), str) + and isinstance(tokens.get("refresh_token"), str), "OAuth tokens missing") + return tokens + + def refresh(self, client: dict, refresh_token: str) -> tuple[int, dict]: + status, _, raw = self.request("/oauth/token", "POST", urllib.parse.urlencode({ + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": client["client_id"], + "client_secret": client["client_secret"], + "resource": self.origin + "/mcp", + }).encode()) + try: + return status, json.loads(raw) + except json.JSONDecodeError: + return status, {"raw": raw.decode(errors="replace")} + + def mcp_status(self, token: str) -> int: + self.rpc_id += 1 + headers = { + "Accept": "application/json, text/event-stream", + "Content-Type": "application/json", + "MCP-Protocol-Version": MCP_PROTOCOL, + "MCP-Method": "tools/list", + "Authorization": "Bearer " + token, + } + status, _, _ = self.request( + "/mcp", "POST", + json.dumps({"jsonrpc": "2.0", "id": self.rpc_id, "method": "tools/list", + "params": {"_meta": { + "io.modelcontextprotocol/protocolVersion": MCP_PROTOCOL, + "io.modelcontextprotocol/clientCapabilities": {}, + }}}).encode(), + headers=headers, + ) + return status + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--bin-dir", required=True) + args = parser.parse_args() + bin_dir = Path(args.bin_dir).resolve() + check((bin_dir / "webcodex").is_file(), f"webcodex binary missing under {bin_dir}") + check(sys.platform != "win32", "POSIX-only harness") + root = Path(tempfile.mkdtemp(prefix="webcodex-oauth-restart-")) + project = root / "project" + make_git_repo(project) + share = Share(bin_dir, root) + try: + share.launch(project, persist=True, tag="persist-a") + first = share.wait_ready() + tokens = share.authorize(first) + check(share.mcp_status(tokens["access_token"]) == 200, + "authorized MCP request failed before restart") + share.stop() + + share.launch(project, persist=True, tag="persist-b") + second = share.wait_ready() + check(second["client_id"] == first["client_id"], + "restart changed the persisted client id") + check(second["client_secret"] == first["client_secret"], + "restart changed the persisted client secret") + check(share.mcp_status(tokens["access_token"]) == 200, + "access token stopped working after a persisting restart") + status, refreshed = share.refresh(first, tokens["refresh_token"]) + check(status == 200, f"refresh grant denied after restart: {status} {refreshed}") + check(isinstance(refreshed.get("access_token"), str), + "refresh response missing access token") + check(share.mcp_status(refreshed["access_token"]) == 200, + "refreshed access token was rejected after restart") + share.stop() + + share.launch(project, persist=False, tag="default-c") + third = share.wait_ready() + check(third["client_id"] == first["client_id"], + "default-mode restart changed the persisted client id") + check(share.mcp_status(tokens["access_token"]) == 401, + "default-mode restart kept a pre-restart access token alive") + status, refused = share.refresh(first, tokens["refresh_token"]) + check(status != 200, + f"default-mode restart kept a pre-restart refresh token alive: {refused}") + share.stop() + + print("PASS oauth restart persistence: client continuity, grant continuity, " + "refresh rotation, default-mode revocation") + shutil.rmtree(root, ignore_errors=True) + return 0 + finally: + share.stop() + if root.exists(): + print(f"retained failed-run evidence under {root}", file=sys.stderr) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/auth/mod.rs b/src/auth/mod.rs index bd5877f34..47498dca9 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -68,8 +68,8 @@ pub(crate) use project_credential::{ pub(crate) use project_share::{ configured_project_share_subject, generate_project_share_session_id, parse_project_share_subject_id, project_share_scopes_are_bounded, - validate_project_share_grant_subject, PROJECT_SHARE_OAUTH_SCOPES, - PROJECT_SHARE_OAUTH_SUBJECT_KIND, PROJECT_SHARE_OAUTH_TOKEN_KIND, + project_share_session_id_for_client, validate_project_share_grant_subject, + PROJECT_SHARE_OAUTH_SCOPES, PROJECT_SHARE_OAUTH_SUBJECT_KIND, PROJECT_SHARE_OAUTH_TOKEN_KIND, }; pub use scopes::{ diff --git a/src/auth/project_share.rs b/src/auth/project_share.rs index 4e70f7aeb..0f8ba6645 100644 --- a/src/auth/project_share.rs +++ b/src/auth/project_share.rs @@ -32,6 +32,22 @@ pub(crate) fn generate_project_share_session_id() -> String { format!("{PROJECT_SHARE_SESSION_PREFIX}{random}") } +/// Restart-stable session id derived from an already validated, persisted +/// project-share OAuth client id. An arbitrary caller string is never an +/// authority input; only the canonical generated client id format is accepted. +pub(crate) fn project_share_session_id_for_client(client_id: &str) -> Result { + let suffix = client_id + .strip_prefix("wc_client_") + .filter(|suffix| { + suffix.len() == 64 + && suffix + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + }) + .ok_or_else(|| "invalid persisted project-share OAuth client ID".to_string())?; + Ok(format!("{PROJECT_SHARE_SESSION_PREFIX}{suffix}")) +} + pub(crate) fn configured_project_share_subject( config: &crate::Config, ) -> Result, String> { @@ -68,3 +84,35 @@ pub(crate) fn project_share_scopes_are_bounded(scopes: &str) -> bool { .split_ascii_whitespace() .all(|scope| PROJECT_SHARE_OAUTH_SCOPES.contains(&scope) || scope == "offline_access") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn persisted_client_session_is_stable_without_weakening_subject_validation() { + let client = format!("wc_client_{}", "a".repeat(64)); + let stable = project_share_session_id_for_client(&client).unwrap(); + assert_eq!( + stable, + format!("{PROJECT_SHARE_SESSION_PREFIX}{}", "a".repeat(64)) + ); + assert_eq!( + project_share_session_id_for_client(&client).unwrap(), + stable + ); + assert_ne!(generate_project_share_session_id(), stable); + for invalid in [ + "".to_string(), + "wc_client_short".to_string(), + format!("wc_client_{}", "g".repeat(64)), + format!("wc_client_{}", "A".repeat(64)), + format!("wc_csec_{}", "a".repeat(64)), + ] { + assert!( + project_share_session_id_for_client(&invalid).is_err(), + "{invalid}" + ); + } + } +} diff --git a/src/project_entry.rs b/src/project_entry.rs index fddc632ef..5f0b092f4 100644 --- a/src/project_entry.rs +++ b/src/project_entry.rs @@ -236,7 +236,7 @@ pub(crate) fn parse_options( pub(crate) fn usage() -> &'static str { "Usage: webcodex share [--root PATH] [--profile NAME] [--state-dir PATH] [--json]\n\ [--tunnel cloudflare|openai|none] [--auth bearer|query-token|oauth]\n\ - [--oauth-redirect-uri URL] [--public-url URL] [--no-copy-url]\n\ + [--oauth-redirect-uri URL] [--oauth-persist-on-restart] [--public-url URL] [--no-copy-url]\n\ [--stop-on-stdin-eof]\n\ webcodex status [--root PATH] [--profile NAME] [--state-dir PATH] [--json]\n\ webcodex doctor [--root PATH] [--profile NAME] [--state-dir PATH] [--json]\n\ @@ -258,6 +258,8 @@ starting services. `run` is the explicit foreground local runtime step. Its opti `--console-assets-dir` enables loopback-only development assets for that run.\n\ `--auth query-token` is a temporary share-only convenience for MCP clients that\n\ cannot configure a Bearer header; `--auth oauth` adds project-bound OAuth.\n\ +`--oauth-persist-on-restart` keeps the validated share OAuth client and its\n\ +unexpired, unrevoked grants across ordinary restarts of the same share.\n\ On Windows, explicit `webcodex share` is supported. Managed Cloudflare acquisition is available on Windows x64; Windows ARM64 requires a trusted explicit/PATH cloudflared because the pinned upstream release has no official ARM64 artifact. Native OpenAI Tunnel supports Windows x64/arm64.\n" } diff --git a/src/project_entry_share.rs b/src/project_entry_share.rs index 6de7a10a5..c0b8e237e 100644 --- a/src/project_entry_share.rs +++ b/src/project_entry_share.rs @@ -49,6 +49,7 @@ pub(crate) struct ShareCommandOptions { pub(crate) tunnel: TunnelProvider, pub(crate) auth: ShareAuth, pub(crate) oauth_redirect_uri: Option, + pub(crate) oauth_persist_on_restart: bool, pub(crate) public_url: Option, pub(crate) copy_url: bool, pub(crate) stop_on_stdin_eof: bool, @@ -58,6 +59,7 @@ pub(crate) fn parse_share_options(args: &[String]) -> Result Result oauth_persist_on_restart = true, "--no-copy-url" => copy_url = false, "--stop-on-stdin-eof" => stop_on_stdin_eof = true, flag => project_args.push(flag.to_string()), @@ -145,6 +148,9 @@ pub(crate) fn parse_share_options(args: &[String]) -> Result Result Result { crate::oauth_http::validate_redirect_uri(redirect_uri).map_err(|message| { share_oauth_state_error(format!("invalid OAuth redirect URI: {message}")) @@ -413,7 +421,12 @@ fn prepare_share_oauth_client( share_oauth_state_error("WebCodex could not restore the share OAuth client") })?; } - revoke_previous_share_oauth_grants(&db, &client_id)?; + // An ordinary restart retires the previous run's grants. Persistence is + // an explicit opt-in: expiry, revocation and token replay rules still + // decide whether a retained grant remains usable. + if !persist_on_restart { + revoke_previous_share_oauth_grants(&db, &client_id)?; + } return Ok(ShareOAuthClient { client_id, client_secret, @@ -630,14 +643,23 @@ pub(crate) async fn share(options: &ShareCommandOptions) -> Result<(), ProductEr .oauth_redirect_uri .as_deref() .ok_or_else(|| share_oauth_state_error("OAuth redirect URI is missing"))?, + options.oauth_persist_on_restart, )?), }; let project_share_oauth = oauth_client .as_ref() - .map(|_| ProjectShareOAuthRuntimeOptions { - project_grant_id: config.project_grant_id(&paths), - session_id: crate::auth::generate_project_share_session_id(), - }); + .map(|client| { + Ok(ProjectShareOAuthRuntimeOptions { + project_grant_id: config.project_grant_id(&paths), + session_id: if options.oauth_persist_on_restart { + crate::auth::project_share_session_id_for_client(&client.client_id) + .map_err(share_oauth_state_error)? + } else { + crate::auth::generate_project_share_session_id() + }, + }) + }) + .transpose()?; // All process/network readiness for this share uses one absolute budget. // Managed binary acquisition and local state setup above are preflight, not @@ -1443,6 +1465,171 @@ mod tests { assert!(oauth.contains("OAuth client secrets are intentionally excluded")); } + #[test] + fn oauth_persist_on_restart_is_an_explicit_oauth_only_choice() { + let oauth = parse_share_options(&[ + "--auth".to_string(), + "oauth".to_string(), + "--oauth-redirect-uri".to_string(), + "https://client.example/callback".to_string(), + ]) + .unwrap(); + assert!(!oauth.oauth_persist_on_restart); + let persistent = parse_share_options(&[ + "--auth".to_string(), + "oauth".to_string(), + "--oauth-redirect-uri".to_string(), + "https://client.example/callback".to_string(), + "--oauth-persist-on-restart".to_string(), + ]) + .unwrap(); + assert!(persistent.oauth_persist_on_restart); + assert!(parse_share_options(&["--oauth-persist-on-restart".to_string()]).is_err()); + assert!(parse_share_options(&[ + "--auth".to_string(), + "bearer".to_string(), + "--oauth-persist-on-restart".to_string(), + ]) + .is_err()); + } + + #[test] + fn project_share_restart_preserves_client_and_only_retires_default_mode_grants() { + let temp = tempfile::tempdir().unwrap(); + let state = temp.path().join("project-state"); + let data = state.join("data"); + fs::create_dir_all(&data).unwrap(); + let paths = ProjectPaths { + data, + credentials: state.join("credentials"), + project_registry: state.join("project-registry"), + logs: state.join("logs"), + config: state.join("project.toml"), + bootstrap_key: state.join("bootstrap-key"), + project_credential: state.join("project-credential"), + agent_token: state.join("agent-token"), + runner_config: state.join("runner-config"), + state, + }; + let config = ProjectConfig { + version: 1, + project_name: "test-project".to_string(), + root: temp.path().join("project"), + profile: "default".to_string(), + port: 24444, + executor_project_id: "test-project".to_string(), + executor_client_id: "test-client".to_string(), + }; + let redirect = "https://client.example/callback"; + let first = prepare_share_oauth_client(&config, &paths, redirect, true).unwrap(); + let stable = crate::auth::project_share_session_id_for_client(&first.client_id).unwrap(); + let db = crate::Database::open(&paths.data.join("webcodex.db")).unwrap(); + let now = chrono::Utc::now().timestamp(); + let access = crate::auth::generate_oauth_access_token(); + let refresh = crate::auth::generate_oauth_refresh_token(); + let revoked = crate::auth::generate_oauth_access_token(); + let subject = format!("{}|{stable}", config.project_grant_id(&paths)); + db.insert_oauth_access_token(&crate::models::OAuthAccessTokenRecord { + admin_authority: false, + id: uuid::Uuid::new_v4().to_string(), + token_hash: crate::auth::hash_token(&access), + client_id: first.client_id.clone(), + subject_kind: crate::auth::PROJECT_SHARE_OAUTH_SUBJECT_KIND.to_string(), + subject_id: subject.clone(), + user_id: None, + scopes: "runtime:read".to_string(), + resource: None, + shared_key_hash: None, + created_at: now, + expires_at: now + 3600, + revoked_at: None, + last_used_at: None, + }) + .unwrap(); + db.insert_oauth_access_token(&crate::models::OAuthAccessTokenRecord { + admin_authority: false, + id: uuid::Uuid::new_v4().to_string(), + token_hash: crate::auth::hash_token(&revoked), + client_id: first.client_id.clone(), + subject_kind: crate::auth::PROJECT_SHARE_OAUTH_SUBJECT_KIND.to_string(), + subject_id: subject.clone(), + user_id: None, + scopes: "runtime:read".to_string(), + resource: None, + shared_key_hash: None, + created_at: now, + expires_at: now + 3600, + revoked_at: Some(now), + last_used_at: None, + }) + .unwrap(); + db.insert_oauth_refresh_token(&crate::models::OAuthRefreshTokenRecord { + admin_authority: false, + id: uuid::Uuid::new_v4().to_string(), + token_hash: crate::auth::hash_token(&refresh), + client_id: first.client_id.clone(), + subject_kind: crate::auth::PROJECT_SHARE_OAUTH_SUBJECT_KIND.to_string(), + subject_id: subject, + user_id: None, + scopes: "runtime:read".to_string(), + resource: None, + shared_key_hash: None, + created_at: now, + expires_at: now + 3600, + revoked_at: None, + last_used_at: None, + rotated_from_id: None, + }) + .unwrap(); + drop(db); + + // Repeated persisting restarts keep one validated client, one stable + // session identity and the still-valid grants. + for _ in 0..2 { + let restarted = prepare_share_oauth_client(&config, &paths, redirect, true).unwrap(); + assert_eq!(restarted.client_id, first.client_id); + assert_eq!(restarted.client_secret, first.client_secret); + assert_eq!( + crate::auth::project_share_session_id_for_client(&restarted.client_id).unwrap(), + stable + ); + } + let db = crate::Database::open(&paths.data.join("webcodex.db")).unwrap(); + assert_eq!( + db.get_oauth_access_token_by_hash(&crate::auth::hash_token(&access)) + .unwrap() + .unwrap() + .revoked_at, + None + ); + assert_eq!( + db.get_oauth_refresh_token_by_hash(&crate::auth::hash_token(&refresh)) + .unwrap() + .unwrap() + .revoked_at, + None + ); + // A persisting restart never makes a revoked grant usable again; the + // store only resolves unrevoked tokens, so the revoked one stays absent. + assert!(db + .get_oauth_access_token_by_hash(&crate::auth::hash_token(&revoked)) + .unwrap() + .is_none()); + drop(db); + + // Switching back to the default mode retires the retained grants. + prepare_share_oauth_client(&config, &paths, redirect, false).unwrap(); + let db = crate::Database::open(&paths.data.join("webcodex.db")).unwrap(); + assert!(db + .get_oauth_access_token_by_hash(&crate::auth::hash_token(&access)) + .unwrap() + .is_none()); + assert!(db + .get_oauth_refresh_token_by_hash(&crate::auth::hash_token(&refresh)) + .unwrap() + .is_none()); + } + #[test] fn machine_share_ready_event_never_contains_handoff_secret_material() { let cloudflare = machine_share_ready_event( From cef471e149c8002aff17f7dade190743968da999 Mon Sep 17 00:00:00 2001 From: vincent Date: Thu, 29 Oct 2026 04:42:42 +1000 Subject: [PATCH 2/2] test(share): prove revocation, replay and transport fences in the OAuth restart E2E Extend the loopback share harness beyond client/grant continuity: replay a rotated refresh token, revoke a refreshed access token and restart again, keep a non-OAuth bearer share working, and show Runner transport paths reject a Server OAuth access token. --- scripts/e2e_share_oauth_restart.py | 93 +++++++++++++++++++++++++----- 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/scripts/e2e_share_oauth_restart.py b/scripts/e2e_share_oauth_restart.py index 85cc81bae..64dc5b67e 100755 --- a/scripts/e2e_share_oauth_restart.py +++ b/scripts/e2e_share_oauth_restart.py @@ -72,40 +72,49 @@ def __init__(self, bin_dir: Path, root: Path) -> None: self.port: int | None = None self.process: subprocess.Popen | None = None self.log_path = root / "share.log" + self.state_dir = root / "state" self.rpc_id = 0 - def launch(self, project_dir: Path, persist: bool, tag: str) -> None: + def launch(self, project_dir: Path, persist: bool, tag: str, auth: str = "oauth", + state_dir: Path | None = None, public_url: bool = True) -> None: log_path = self.root / f"share-{self.public_port}-{tag}.log" log_path.write_bytes(b"") self.log_path = log_path + self.state_dir = state_dir or self.root / "state" command = [ str(self.bin_dir / "webcodex"), "share", "--tunnel", "none", - "--public-url", self.origin, - "--auth", "oauth", - "--oauth-redirect-uri", f"{self.origin}/callback", "--no-copy-url", - "--state-dir", str(self.root / "state"), + "--state-dir", str(self.state_dir), ] - if persist: - command.append("--oauth-persist-on-restart") + if public_url: + command += ["--public-url", self.origin] + command += ["--auth", auth] + if auth == "oauth": + command += ["--oauth-redirect-uri", f"{self.origin}/callback"] + if persist: + command.append("--oauth-persist-on-restart") log = open(log_path, "ab") self.process = subprocess.Popen( command, cwd=project_dir, stdout=log, stderr=log, start_new_session=True ) def discover_server_port(self) -> None: - configs = list((self.root / "state").glob("**/runner.toml")) + configs = list(self.state_dir.glob("**/runner.toml")) check(bool(configs), "generated Runner config not found under state dir") match = re.search(r'server_url\s*=\s*"http://127\.0\.0\.1:(\d+)"', configs[0].read_text()) check(bool(match), f"server_url missing from {configs[0]}") self.port = int(match.group(1)) - def wait_ready(self, deadline_secs: float = 90.0) -> dict: + def wait_ready(self, deadline_secs: float = 90.0, oauth: bool = True) -> dict: deadline = time.monotonic() + deadline_secs ready_re = re.compile( - r"4\. Client ID: (?P\S+)\n.*?5\. Client secret: (?P\S+)\n" - r".*?Project share credential \(this share only\): (?P\S+)", + ( + r"4\. Client ID: (?P\S+)\n.*?5\. Client secret: (?P\S+)\n" + r".*?Project share credential \(this share only\): (?P\S+)" + ) + if oauth + else r"Credential \(this share only\): (?P\S+)", re.S, ) while time.monotonic() < deadline: @@ -116,6 +125,8 @@ def wait_ready(self, deadline_secs: float = 90.0) -> dict: match = ready_re.search(self.log_path.read_text(errors="replace")) if match: self.discover_server_port() + if not oauth: + return {"credential": match.group("credential")} return { "client_id": match.group("client"), "client_secret": match.group("secret"), @@ -212,6 +223,25 @@ def refresh(self, client: dict, refresh_token: str) -> tuple[int, dict]: except json.JSONDecodeError: return status, {"raw": raw.decode(errors="replace")} + def revoke(self, client: dict, token: str, hint: str) -> int: + status, _, _ = self.request("/oauth/revoke", "POST", urllib.parse.urlencode({ + "token": token, + "token_type_hint": hint, + "client_id": client["client_id"], + "client_secret": client["client_secret"], + }).encode()) + return status + + def runner_transport_status(self, token: str) -> int: + status, _, _ = self.request( + "/api/shell/agent/poll", "POST", b"{}", + headers={ + "Authorization": "Bearer " + token, + "Content-Type": "application/json", + }, + ) + return status + def mcp_status(self, token: str) -> int: self.rpc_id += 1 headers = { @@ -260,12 +290,31 @@ def main() -> int: "restart changed the persisted client secret") check(share.mcp_status(tokens["access_token"]) == 200, "access token stopped working after a persisting restart") + check(share.runner_transport_status(tokens["access_token"]) in (401, 403), + "Runner transport accepted a Server OAuth access token") status, refreshed = share.refresh(first, tokens["refresh_token"]) check(status == 200, f"refresh grant denied after restart: {status} {refreshed}") check(isinstance(refreshed.get("access_token"), str), "refresh response missing access token") + check(isinstance(refreshed.get("refresh_token"), str), + "refresh response missing rotated refresh token") check(share.mcp_status(refreshed["access_token"]) == 200, "refreshed access token was rejected after restart") + status, replayed = share.refresh(first, tokens["refresh_token"]) + check(status != 200, + f"rotated refresh token was accepted twice after restart: {replayed}") + check(share.revoke(first, refreshed["access_token"], "access_token") == 200, + "revocation of the refreshed access token was not acknowledged") + check(share.mcp_status(refreshed["access_token"]) == 401, + "revoked access token still worked") + share.stop() + + share.launch(project, persist=True, tag="persist-c") + share.wait_ready() + check(share.mcp_status(refreshed["access_token"]) == 401, + "a persisting restart resurrected a revoked access token") + check(share.mcp_status(tokens["access_token"]) == 200, + "unrevoked access token did not survive a persisting restart") share.stop() share.launch(project, persist=False, tag="default-c") @@ -274,13 +323,31 @@ def main() -> int: "default-mode restart changed the persisted client id") check(share.mcp_status(tokens["access_token"]) == 401, "default-mode restart kept a pre-restart access token alive") - status, refused = share.refresh(first, tokens["refresh_token"]) + status, refused = share.refresh(first, refreshed["refresh_token"]) check(status != 200, f"default-mode restart kept a pre-restart refresh token alive: {refused}") share.stop() + # A non-OAuth share keeps working, and the persist flag stays OAuth-only. + share.launch(project, persist=False, tag="bearer", auth="bearer", public_url=False, + state_dir=root / "state-bearer") + bearer = share.wait_ready(oauth=False) + check(share.mcp_status(bearer["credential"]) == 200, + "non-OAuth share rejected its own project credential") + share.stop() + rejected = subprocess.run( + [str(bin_dir / "webcodex"), "share", "--tunnel", "none", "--no-copy-url", + "--oauth-persist-on-restart", "--state-dir", str(root / "state-flag")], + cwd=project, capture_output=True, text=True, timeout=60, + ) + check(rejected.returncode != 0 + and "requires --auth oauth" in (rejected.stdout + rejected.stderr), + "persist flag without --auth oauth was not rejected: " + + rejected.stdout + rejected.stderr) + print("PASS oauth restart persistence: client continuity, grant continuity, " - "refresh rotation, default-mode revocation") + "refresh rotation one-shot, revocation not resurrected, default-mode " + "revocation, non-OAuth share unaffected, Runner transport rejects OAuth") shutil.rmtree(root, ignore_errors=True) return 0 finally: