Security hardening: client input rules, server-made thread ids, per-thread run lock and production defaults

CLAUDE.md

@@ -96,8 +96,9 @@ Parsed by `tenxgraph_api/src/app/core/config/graph_config.py`. Supported keys:
 | `rate_limit` | Object (see below) |
 | `ag_ui` | `{"enabled": bool}`, default off. Mounts `POST /v1/ag-ui` (AG-UI protocol, for CopilotKit and other AG-UI clients). Needs the `ag-ui` extra |
 | `websocket` | `enabled` (`false` leaves out `/v1/graph/ws` and `/v1/graph/live`; `true` without `websockets`/`wsproto` refuses to boot; unset keeps them and warns if no library; `routers/setup_router.py::_graph_router`), `max_connections` (1000), `max_connections_per_user` (10), `realtime_models` (models a `/v1/graph/live` client may pick), `realtime_client_overrides` (init options a client may set; default `voice`, `modalities`, `vad`; add `system_prompt` only for trusted callers) |
-| `runs` | `RunsConfig`: `thread_ids` (`client` default, `server` = only existing ids may be named, enforced by the ownership/rbac backends), `config_keys` / `state_keys` (allow-lists for request config and `initial_state`/PUT state; 422 on other keys; `thread_id`, `run_id` always allowed), `max_concurrent_per_user` (HTTP invoke/stream/AG-UI runs in flight, default 10, 429 over it; `routers/graph/run_limits.py`), `timeout` (seconds; invoke 504, streams end with an error chunk) |
-| `token_revocation` | `"redis"` / `"memory"` / `"module:attr"` -> `core/auth/revocation.py::TokenRevocationBackend`, checked by `RequirePermission` (401) and before every WS run; backend errors fail closed (503) |
+| `runs` | `RunsConfig`: `thread_ids` (`server` default = only existing ids may be named on REST invoke/stream, `/v1/graph/ws` and checkpointer writes, a new thread is started by omitting the id; enforced by the ownership/rbac backends via `OwnershipAuthorizationBackend(server_thread_ids=...)`. AG-UI and `/v1/graph/live` are exempt: they pass `client_thread_ids=True` to `authorize` because their frontends make their own ids, and the caller owns a new one. `client` = any request may name a new thread, explicit opt-in), `config_keys` / `state_keys` (allow-lists for request config and `initial_state`/PUT state; 422 on other keys; `thread_id`, `run_id` always allowed), `max_concurrent_per_user` (HTTP invoke/stream/AG-UI runs in flight, default 10, 429 over it; `routers/graph/run_limits.py`), `timeout` (seconds; invoke 504, streams end with an error chunk), `max_parallel_tool_calls` (positive int, default unset = core's default 10 or env `TENXGRAPH_MAX_PARALLEL_TOOL_CALLS`; set into every run's config in `_prepare_input`, reserved from clients) |
+| `token_revocation` | `"redis"` / `"memory"` / `"module:attr"` -> `core/auth/revocation.py::TokenRevocationBackend`, checked by `RequirePermission` (401), before every `/v1/graph/ws` run, and before and every 30 s during a `/v1/graph/live` session (`REALTIME_RECHECK_INTERVAL`, close 1008); backend errors fail closed (503) |
+| `media` | `delete_with_thread` (default true): delete an uploaded file when the last message or thread that references it is deleted. Links are recorded in the file's `threads` metadata (`MediaService.link_threads` / `unlink_threads`); files without a `threads` key (uploaded before link tracking) and stores without `update_metadata` (core older than the release that added it) are never auto-deleted. `false` still records links. Parsed by `graph_config.MediaConfig` |
 | `graph_introspection` | `"open"` or `"scoped"` (default scoped in production): `GET /v1/graph`, `/v1/graph/tools`, `/v1/graph:StateSchema` need an explicit `graph:read` scope (`RequirePermission(..., introspection=True)`) |
 
 `rate_limit` object: `enabled`, `requests` (default 100), `window` secs (60), `by` (`ip` |
@@ -122,10 +123,18 @@ and for redis backend a `redis` sub-object `{ "url", "prefix" }` (or shorthand U
   `allow_all` only with auth off in development (no identity, so nothing to authorize).
   Selection lives in `loader._resolve_authorization_backend`; `RequirePermission` passes
   `resource_id` (thread_id from path or body) to `authorize`.
+- **Isolation defaults to owner.** `AuthorizationBackend.isolation_scope()` returns `"owner"`, so a
+  custom backend with only `authorize()` still scopes threads and memories per user; return
+  `"none"` on purpose to share (`DefaultAuthorizationBackend` / `allow_all` does).
+- **Token claims are validated.** `JwtAuth` requires `user_id` to be a non-empty string or an int
+  (stored as a string), else 401. `token_revoked` checks `jti` even with an odd `user_id`.
+  Scopes come from `authorization.scopes_from_claims`: the `scope` claim (space-separated) and
+  `scopes` (list or space-separated string) merge into exact names, checked with `in` (no
+  substrings); a wrong-typed claim is a 401 (`ScopeClaimError`), no claim means unrestricted.
 - **Ownership fails closed.** No checkpointer, or one whose `aget_thread_owner` raises
   `NotImplementedError`, denies thread requests; with auth on the loader refuses to boot in that
   setup (`authorization.assert_ownership_supported`). A thread with no owner allows only
-  `CREATE_ACTIONS` (`graph:invoke`, `graph:stream`, `checkpointer:write`); checkpointer state and
+  `CREATE_ACTIONS` (`graph:invoke`, `graph:stream`, `checkpointer:write`, `checkpointer:import`); checkpointer state and
   message writes claim the thread first (`CheckpointerService._claim_thread`). Every saved
   thread carries its owner via `request_config.owned_thread_info` (the in-memory and SQLite
   checkpointers read the owner from `ThreadInfo.user_id`).
@@ -154,12 +163,51 @@ and for redis backend a `redis` sub-object `{ "url", "prefix" }` (or shorthand U
   `POST /v1/graph/stop`, `POST /v1/graph/fix`, `GET /v1/graph`,
   `WS /v1/graph/ws`.
 - **Checkpointer / threads**: `GET/POST /v1/threads`, `GET/DELETE /v1/threads/{thread_id}`,
-  `GET /v1/threads/{thread_id}/state`, `GET /v1/threads/{thread_id}/messages`,
-  `... /messages/{message_id}`.
+  `GET/PUT/DELETE /v1/threads/{thread_id}/state`, `GET /v1/threads/{thread_id}/messages`,
+  `GET/DELETE ... /messages/{message_id}`. `POST /v1/threads/{thread_id}/messages` (import
+  history, any role) is admin-only: `RequirePermission("checkpointer", "import",
+  explicit_scope=True)`, so the caller must hold `checkpointer:import` explicitly (no scopes
+  means 403; `"*"` roles get it, `authorization.CHECKPOINTER_ADMIN_SCOPES`). Tool calls, media
+  ownership and size limits still apply. `PUT ... /messages/{message_id}` sets
+  `metadata["feedback"]` only (body `{"feedback": {...}}`, 8 KB, depth 10, `checkpointer:write`;
+  calls `aupdate_message_metadata`, 501 on cores older than 0.10.4, 404 for a missing message).
+- **Client input rules** (`core/auth/request_config.py`): `context_summary` and `execution_meta`
+  are never client-writable (`reject_builtin_state_keys`, 422); client messages are user-only
+  (`client_message_error`: no `assistant`/`system`; a `tool` message only as tool results, which
+  must answer a pending remote call, `GraphService._check_tool_results`, also for PUT state
+  `context`); the server replaces client `message_id`s (`with_fresh_message_ids`, in the schema
+  validators; AG-UI validates with `context={KEEP_MESSAGE_IDS: True}` to keep its ids);
+  `current_node` must be a real node and state values must match the state model's types (422).
+  Every external media URL in client input (`media.kind="url"`, not `graph://media/` or
+  `data:`; `gs://` included) is run through the graph's `media_policy.check_url` before the run
+  (`multimodal_preprocessor.check_external_media_urls`, via `GraphService.check_media_urls`,
+  also called by `CheckpointerService` for `POST messages` and `PUT state`): 400 with the policy
+  message, `MediaURLBlockedError`. `resolve_media_policy` uses `compiled_graph.media_policy`,
+  else a default `MediaFetchPolicy`, else skips (core without it). The policy lives in core;
+  the API has no config for it.
+- **Per-thread run lock** (`routers/graph/thread_lock.py`): invoke, stream, each WS run and
+  AG-UI take a one-slot lease `runs:thread:<thread id>` from `core/concurrency.py` (Redis when
+  configured, else per process; 60 s lease renewed while alive) before the graph starts, and
+  release it in `finally` (error, cancel, disconnect via `release_when_done`). Routers lock when
+  the request names a thread (streaming routes before the response headers, so the 409 is
+  real); `GraphService` locks a thread it creates (`_lock_new_thread`). Busy: wait up to
+  `THREAD_BUSY_WAIT_SECONDS` (30), then 409 "thread is busy" in the standard error envelope (WS: error chunk
+  `data.reason = "thread is busy"`, socket stays open). Request option `on_busy`
+  (`"wait"` | `"cancel"`, top level next to `recursion_limit`; not on AG-UI): `cancel` calls
+  `GraphService.stop_graph` while waiting. `/v1/graph/live` is not locked.
+- **Production error chunks:** `graph_service.sanitize_error_chunk` (called in
+  `stream_chunks`, so stream, WS and AG-UI share it) replaces `data.reason` / `data.error` /
+  `data.message` and `state.execution_meta.internal_data["error"]` of `ERROR` chunks with the
+  generic `_sanitize_error_message` text when `MODE=production`. Core's
+  `... internal error (reference <id>).` tool text is kept.
 - **Store (memory)**: `POST /v1/store/memories`, `/v1/store/memories/list`,
   `/v1/store/memories/forget`, `/v1/store/memories/{memory_id}`, `POST /v1/store/search`.
-- **Media / files** (`tags=["Files"]`): `POST /v1/files/upload`, `GET /v1/files/{file_id}`,
-  `/{file_id}/info`, `/{file_id}/url`, `GET /v1/config/multimodal`.
+- **Media / files** (`tags=["Files"]`): `POST /v1/files/upload` (first bytes must match the
+  declared type for formats with a signature, else 415; table in `utils/media/signatures.py`),
+  `GET /v1/files/{file_id}`, `/{file_id}/info`, `/{file_id}/url`, `DELETE /v1/files/{file_id}`
+  (owner only, 404 otherwise, scope `files:delete`, added in `authorization.FILES_DELETE_SCOPES`
+  until core lists it), `GET /v1/config/multimodal`. Deleting a message or thread also releases
+  the files it referenced (`CheckpointerService.delete_message` / `delete_thread`).
 - **AG-UI** (`tags=["AG-UI"]`, only when `ag_ui.enabled`): `POST /v1/ag-ui`. `routers/ag_ui/`:
   `converter.py` (RunAgentInput -> new user/tool messages only; the checkpoint is the record),
   `event_mapper.py` (StreamChunk -> AG-UI events), `service.py` (runs `GraphService.stream_chunks`).
@@ -177,8 +225,9 @@ were removed in the 0.5.0 readiness pass; they were fully commented out and neve
 ## Settings / environment
 
 `core/config/settings.py` is a `pydantic-settings` `Settings` (with `extra="allow"`, so unknown
-env vars are tolerated). Notable vars: `APP_NAME`, `APP_VERSION`, `MODE` (`development` |
-`production`), `LOG_LEVEL`, `IS_DEBUG`, `MAX_REQUEST_SIZE` (10MB default), security headers
+env vars are tolerated). Notable vars: `APP_NAME`, `APP_VERSION`, `MODE` (`production` | `prod` |
+`development` | `dev` | `local`, case-insensitive, normalised to `production` or `development`;
+anything else refuses to start, unset is `development`), `LOG_LEVEL`, `IS_DEBUG`, `MAX_REQUEST_SIZE` (10MB default), security headers
 (`SECURITY_HEADERS_ENABLED`, `HSTS_*`, `FRAME_OPTIONS`, `CSP_POLICY`, ...), `ORIGINS` (CORS,
 default `*` with a wildcard warning), `ALLOWED_HOST`, `ROOT_PATH`/`DOCS_PATH`/`REDOCS_PATH`,
 `REDIS_URL`, `SENTRY_DSN`, `SNOWFLAKE_*` (epoch/node/worker/bit layout), `JWT_SECRET_KEY`/

SECURITY.md

@@ -75,8 +75,8 @@ CORS combined with credentials in production. For anything else, in production s
   it to `allow_all`)
 - `STRICT_PRODUCTION_CHECKS=true` to refuse to start on wildcard `ALLOWED_HOST` or `ORIGINS`
   or enabled API docs
-- `runs.thread_ids: "server"` if thread ids could be guessed, so nobody can claim an id
-  before the user it was meant for
+- `runs.thread_ids` at its default `"server"`, so nobody can claim an id before the user it
+  was meant for (AG-UI and `/v1/graph/live` still accept frontend-made ids; `"client"` opts out)
 - `runs.config_keys` / `runs.state_keys` if a tool or node trusts a value such as `role` or
   `plan` from request config or state; without them a client can send any non-reserved key
 - `runs.timeout` and `runs.max_concurrent_per_user` to bound what one account can hold open

tenxgraph_api/cli/commands/api.py

@@ -25,6 +25,7 @@
 from tenxgraph_api.cli.core.config import ConfigManager
 from tenxgraph_api.cli.core.validation import validate_cli_options
 from tenxgraph_api.cli.exceptions import ConfigurationError, ServerError
+from tenxgraph_api.src.app.core.config.settings import normalize_mode_value
 
 
 # Mirrors Settings.MAX_REQUEST_SIZE; read from the environment the command just loaded.
@@ -109,6 +110,12 @@ def execute(
                         load_dotenv()
                         step.detail("environment: process defaults")
 
+                    # Fail early with a clear message instead of a settings traceback.
+                    try:
+                        normalize_mode_value(os.environ.get("MODE"))
+                    except ValueError as e:
+                        raise ConfigurationError(str(e)) from e
+
                     os.environ["GRAPH_PATH"] = str(actual_config_path)
 
                     # Add project root to sys.path for importing graph modules

tenxgraph_api/cli/commands/build.py

@@ -11,10 +11,12 @@
 from tenxgraph_api.cli.exceptions import DockerError, FileOperationError, ValidationError
 from tenxgraph_api.cli.templates.defaults import (
     CLI_PACKAGE,
+    SECRET_FILE_PATTERNS,
     generate_docker_compose_content,
     generate_dockerfile_content,
     generate_dockerignore_content,
     generate_k8s_manifest_content,
+    merge_ignore_patterns,
 )
 
 
@@ -121,7 +123,15 @@ def execute(  # noqa: PLR0912, PLR0915
                 dockerignore_path = output_path.parent / ".dockerignore"
                 with timeline.step("dockerignore") as step:
                     if dockerignore_path.exists() and not force:
-                        step.skip(f"{dockerignore_path} already exists")
+                        # Keep the developer's file, but never let it ship credentials
+                        merged = merge_ignore_patterns(
+                            dockerignore_path.read_text(encoding="utf-8"), SECRET_FILE_PATTERNS
+                        )
+                        if merged is None:
+                            step.skip(f"{dockerignore_path} already exists")
+                        else:
+                            dockerignore_path.write_text(merged, encoding="utf-8")
+                            step.detail(f"{dockerignore_path}: added missing secret patterns")
                     else:
                         dockerignore_path.write_text(
                             generate_dockerignore_content(), encoding="utf-8"

tenxgraph_api/cli/commands/init.py

@@ -11,6 +11,11 @@
 from tenxgraph_api.cli.commands import BaseCommand
 from tenxgraph_api.cli.core.prompts import Choice
 from tenxgraph_api.cli.exceptions import FileOperationError, ValidationError
+from tenxgraph_api.cli.templates.defaults import (
+    GITIGNORE_SECRET_PATTERNS,
+    generate_gitignore_content,
+    merge_ignore_patterns,
+)
 
 
 _TEMPLATES_DIR = Path(__file__).parent.parent / "templates"
@@ -159,6 +164,7 @@ def execute(  # noqa: PLR0913
                 ]
                 if "10xgraph.json" not in planned:
                     planned.append("10xgraph.json")
+                planned.append(".gitignore")
                 self.output.print_list(planned, title="Files that would be created", bullet="→")
                 self.output.info("Dry run complete; no files were written.", emoji=False)
                 return 0
@@ -186,6 +192,7 @@ def execute(  # noqa: PLR0913
                         on_file=step.detail,
                     )
                     step.detail(f"{len(created)} files from the {template_dir.name} template")
+                    step.detail(self._write_gitignore(base_path))
 
                 with timeline.step("config") as step:
                     # Regenerate 10xgraph.json from the built config. Force is safe only
@@ -635,6 +642,29 @@ def _copy_template_dir(
             created.add(dest)
         return created
 
+    def _write_gitignore(self, base_path: Path) -> str:
+        """Write a .gitignore, or add the secret patterns an existing one lacks.
+
+        The next steps tell the developer to put API keys and JWT_SECRET_KEY in .env, so
+        .env must be ignored. An existing .gitignore is never replaced, even with --force.
+        """
+        path = base_path / ".gitignore"
+        try:
+            if not path.exists():
+                path.write_text(generate_gitignore_content(), encoding="utf-8")
+                return ".gitignore"
+            merged = merge_ignore_patterns(
+                path.read_text(encoding="utf-8"), GITIGNORE_SECRET_PATTERNS
+            )
+            if merged is None:
+                return ".gitignore already ignores .env and credentials"
+            path.write_text(merged, encoding="utf-8")
+            return ".gitignore: added missing secret patterns"
+        except OSError as e:
+            raise FileOperationError(
+                f"Failed to write file {path}: {e}", file_path=str(path)
+            ) from e
+
     def _write_file(self, path: Path, content: str, *, force: bool) -> None:
         try:
             path.parent.mkdir(parents=True, exist_ok=True)

tenxgraph_api/cli/commands/skills.py

@@ -418,6 +418,8 @@ def _install_one(
             )
             for artifact in target.artifacts
         ]
+        for _artifact, _source, dest in installs:
+            self._ensure_inside_project(project_root, dest)
         for _artifact, source, _dest in installs:
             if not source.exists():
                 raise FileOperationError(
@@ -457,6 +459,34 @@ def _install_one(
 
         return installed_paths
 
+    @staticmethod
+    def _ensure_inside_project(project_root: Path, dest: Path) -> None:
+        """Refuse a destination that is, or sits under, a symlink, or that leaves the project.
+
+        A repo can ship ``.claude`` as a symlink to another directory; installing through it
+        would delete and write files outside ``--path``. Every existing component between the
+        project root and ``dest`` is checked, then the resolved path is checked as well.
+        """
+        try:
+            parts = dest.relative_to(project_root).parts
+        except ValueError:
+            parts = None
+        if parts is not None:
+            current = project_root
+            for part in parts:
+                current = current / part
+                if current.is_symlink():
+                    raise FileOperationError(
+                        f"Refusing to install through a symlink: {current}. "
+                        "Replace it with a real directory or file, or pass a different --path.",
+                        file_path=str(current),
+                    )
+        if parts is None or not dest.resolve().is_relative_to(project_root.resolve()):
+            raise FileOperationError(
+                f"Refusing to install outside the project directory: {dest}",
+                file_path=str(dest),
+            )
+
     def _print_activation_hint(self, target: _AgentTarget) -> None:
         """Tell the user how to make the freshly installed skill take effect.