diff --git a/AGENTS.md b/AGENTS.md index 8b5648c..19c1f79 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,57 +1,274 @@ -# AGENTS.md — Multi-agent compatibility shim +# AGENTS.md — Agent Guide for jvspatial -This file exists so AI coding assistants that follow the `AGENTS.md` convention (Codex, Cursor, Aider, Continue, OpenHands, and others) find the same guidance as Claude Code. +> This file is the **canonical entry point** for every AI agent maintaining `jvspatial`. Keep it short, current, and actionable. -**The canonical agent guide is [CLAUDE.md](CLAUDE.md).** Treat it as authoritative. +Keep agent guidance here or in scoped `AGENTS.md` files. Do not create a `CLAUDE.md` shim. -This file does not duplicate that content; agents that read this file should read [CLAUDE.md](CLAUDE.md) instead. +--- + +## What jvspatial is + +`jvspatial` is an **async-first, serverless-compatible, object-spatial Python library** for graph-based persistence and business-logic layers. `Node` and `Edge` inherit from `Object`; `Root` inherits from `Node`; `Walker` is a separate traversal class. The library supports five database backends (JSON, SQLite, MongoDB, DynamoDB, PostgreSQL) and ships a FastAPI integration with auth, file storage, observability, and serverless ergonomics. + +For full positioning and non-goals, read [PRD.md](PRD.md). +For the technical contract, read [SPEC.md](SPEC.md). + +--- + +## Where to look first + +When given a task, resolve context in this order: + +1. **[PRD.md](PRD.md)** — what's the product context? Is this in scope? +2. **[SPEC.md](SPEC.md)** — what does the library currently guarantee about the area you're touching? Every claim cites a `file:line`. +3. **[docs/md/README.md](docs/md/README.md)** — index of how-to docs. Find the relevant one before reading source. +4. **[ROADMAP.md](ROADMAP.md)** — is this area under active hardening? Known gap? Out of scope? +5. **[CHANGELOG.md](CHANGELOG.md)** — recent changes that may affect your work. +6. **Source code** — last, not first. The map of subpackage READMEs (`jvspatial/*/README.md`) shortens this hop. + +If a doc disagrees with code, the doc is wrong. File an issue and trust the code — but cite the discrepancy. --- -## Quick orientation +## Non-negotiable invariants + +These are properties the library guarantees. Breaking them is a regression even if tests pass. + +### 1. Async-only I/O + +Every database call, network call, and file-system call is `async`. There are **no sync wrappers** in the library. Sync is for pure computation only. + +- ❌ `entity.save()` (missing `await`) +- ✅ `await entity.save()` + +If you find yourself wanting a sync wrapper, you are solving the wrong problem. + +### 2. Constant-time secret comparison + +Any comparison of a secret, key, token, or password hash uses `hmac.compare_digest`. Affected paths (SPEC §15.2): +- API key verification, refresh token comparison, password reset token, webhook HMAC, deferred-invoke secret, bcrypt legacy fallback. + +Never replace `hmac.compare_digest` with `==` for performance or readability. + +### 3. Entity name override (`__entity_name__`) + +Persisted entity discriminator is `cls.__dict__.get("__entity_name__") or cls.__name__` (SPEC §1.2). Resolution is **per-subclass**, not inherited. Code that constructs IDs or looks up subclasses must go through `generate_id()` and `find_subclass_by_name()` — never assemble IDs from `cls.__name__` directly. + +### 4. Serverless detection precedence + +`is_serverless_mode(config=None)` resolves: explicit config → current Server config → `SERVERLESS_MODE` env → auto-detection (SPEC §11.1). Do not bypass with custom env reads. Do not memoize results across tests — call `reset_serverless_mode_cache()` between cases. + +### 5. Protected attribute validation + +`Object.__setattr__` validates field names against the class hierarchy. Setting an undeclared attribute on an `Object` is rejected, including an underscore-prefixed name. Declare runtime-only state with Pydantic `PrivateAttr`; a private helper already declared on the class can be replaced on an instance. Never use `object.__setattr__(self, ...)` to bypass the check. + +### 6. Deferred-save MRO + +`class MyEntity(DeferredSaveMixin, Node)` — mixin must come **before** the base. Wrong order raises at subclass creation. Tests should assert MRO-sensitive behavior. + +### 7. Stability tier discipline + +Symbols in `jvspatial.__all__` are public — breaking changes require a deprecation cycle (`docs/md/stability.md`). Underscore-prefixed modules are internal — callers should not import directly. Promoting an internal helper to public is a product decision; update PRD §7 and docs/md/stability.md together. + +### 8. CORS / CSP / docs defaults + +CORS does **not** default to wildcard. CSP is strict on app routes, relaxed only on `/docs`, `/redoc`, `/openapi.json`. `JVSPATIAL_DOCS_DISABLED` is the production posture. Do not weaken these defaults without a security review entry. + +Auth-enabled servers also retain the per-IP 5-request/60-second cap on register, login, forgot-password, and reset-password when global rate limiting is off. `RateLimitConfig(auth_entrypoint_rate_limit_enabled=False)` is an explicit opt-out for tests or hosts with a trusted replacement limiter; do not infer it from the global limiter flag. + +Raw FastAPI routes may defer authentication to FastAPI only when they declare an enforcing `SecurityBase` dependency. Dependency function names do not prove authentication. Admin-only routes and routes with jvspatial auth metadata still use framework authentication and RBAC. + +In-memory `$regex` evaluation has a per-candidate timeout. Preserve that bound when changing query backends or operators; input length limits alone do not prevent backtracking. Webhook requests with idempotency keys must claim a durable record before handler execution and persist an outcome before success is returned. A pending claim needs reconciliation before a new attempt. + +### 9. OAuth signing key custody + +OAuth hosts in production must set `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY` from a secret manager. With `JVSPATIAL_ENVIRONMENT=production`, the keystore refuses to load or create a key without it. The keystore persists encrypted private keys, migrates legacy plaintext keys on read, and fails closed if an encrypted key cannot be decrypted. Keep the same key across workers and restarts; follow [security-operational-notes.md](docs/md/security-operational-notes.md#oauth-signing-key-custody) for backups and rotation. + +### 10. Walker protection + +`max_steps=10000`, `max_visits_per_node=100`, `max_execution_time=300s`, `max_queue_size=1000` are defaults that prevent DOS. Disabling protection is allowed locally; never disable globally or in code that touches user input. + +### 11. Cross-context persistence + +The in-memory matcher and PostgreSQL pushdown treat both missing and JSON `null` as absent for `$exists`; MongoDB retains its native field-presence semantics. `graph_transaction` isolates request identity state and invalidates touched entities in the parent cache after commit or rollback. Index setup must be scoped to the database instance so one store cannot suppress another store's indexes. Preserve these contracts when changing query translation, caches, or transactions. + +File-backed SQLite closes its prior connection when rebinding to another event loop; dropping it without `close()` leaks the `aiosqlite` worker thread and can keep the process alive. -If you cannot follow the link to `CLAUDE.md`, here is the minimum: +--- + +## Common gotchas -- **What this repo is**: `jvspatial`, an async-first Python library for graph-based persistence with FastAPI integration. See [README.md](README.md) and [PRD.md](PRD.md). -- **Where the contract lives**: [SPEC.md](SPEC.md). Every claim cites a `file:line`. If your edit changes a contract, update SPEC in the same commit. -- **How docs are organized**: [docs/md/README.md](docs/md/README.md) is the index. PRD / SPEC / ROADMAP / CLAUDE live at the repo root. -- **Forward direction**: [ROADMAP.md](ROADMAP.md) — current focus areas, known gaps, out-of-scope. -- **What changed recently**: [CHANGELOG.md](CHANGELOG.md). +| Symptom | Cause | Fix | +|---|---|---| +| `TypeError: object NoneType can't be used in 'await' expression` | Forgot `await` on async DB call | Add `await` | +| Custom mixin doesn't batch saves | `DeferredSaveMixin` placed after base in MRO | Mixin first: `class X(DeferredSaveMixin, Node)` | +| Subclass lookup returns wrong class | Two unrelated classes share `__name__` | Set `__entity_name__: ClassVar[Optional[str]] = "Distinct"` on one | +| Tests pass locally, fail under serverless mode | Forgot to test both modes | Use `reset_serverless_mode_cache()` between tests | +| 401 with valid token | Auth state on wrong database | Auth is **always** on prime DB; do not relocate | +| Slow query never logged | `slow_query_ms` not configured | Set in `create_database(observe=True, slow_query_ms=N)` | +| Walker visits the same node forever | Protection disabled | Re-enable `protection_enabled=True` | +| `JVSPATIAL_FOO` env var ignored | Not in allowlist | Add to `jvspatial/env_adapter.py` allowlist | +| SQLite query falls back to in-memory filter | Operator not yet pushed down by `SQLiteTranslator` | Check `jvspatial/db/_sqlite_translate.py` for supported operators | +| `AttributeError` setting `_runtime_field` on an entity | Private state was never declared | Declare it with Pydantic `PrivateAttr`; only declared private helpers may be replaced | +| Auth test receives `429` with global limiter off | Auth entrypoint cap is independent | Explicitly disable the auth cap in test server config, or use a trusted host limiter | --- -## Non-negotiable invariants (summary) +## Graph modeling convention + +Convention for any application built on jvspatial. Not enforced by the library, but required by review and assumed by downstream tooling. -Full list in [CLAUDE.md § Non-negotiable invariants](CLAUDE.md#non-negotiable-invariants). Top items: +1. **Every `Node` must be reachable from `Root`** — directly or through intermediate nodes. An orphaned node is a bug. If a record does not belong in the graph, it must not be a `Node`. +2. **Applications declare an application-root node** — e.g. `class App(Node)`, `class MyApp(Node)`. Create exactly one instance, connect it to `Root`, and treat it as the entry point for all application state. +3. **All application nodes extend from the app-root** — directly or under an appropriate branch / category node beneath it (e.g. `App -> Users -> User`, `App -> Catalog -> Product`). Do not connect application nodes directly to `Root`; `Root` belongs to the library, the app-root belongs to the application. +4. **Use `Object` (not `Node`) for record-style data that does not benefit from graph membership** — audit/change-event records, denormalized snapshots, ledger entries, raw inbound payloads. `ChangeEvent` **must be an `Object`**, not a `Node`. Rule of thumb: if nothing will ever traverse to or from it, it is not a `Node`. -1. Async-only I/O. No sync wrappers. -2. `hmac.compare_digest` for every secret comparison. -3. `__entity_name__` honors per-subclass override; do not assemble IDs from `cls.__name__`. -4. Serverless detection precedence: explicit config → current Server config → `SERVERLESS_MODE` env → auto-detect. -5. Stability tiers in [docs/md/stability.md](docs/md/stability.md) are binding. Public names live in `jvspatial.__all__`; underscore modules are internal. -6. CORS/CSP/docs defaults fail closed; do not weaken without a security review. +When reviewing or generating code, reject `Node` subclasses that are never connected, and reject `Object` subclasses that are used as relationship endpoints. --- ## Run the dev loop ```bash +# install pip install -e '.[dev,test]' pre-commit install -pytest -q -pre-commit run --all-files + +# fast feedback +pytest -q # unit + integration (skips benchmarks) +pre-commit run --all-files # lint / format / type-check + +# full quality bar (run before opening PR) +pytest --cov=jvspatial --cov-report=term-missing + +# benchmarks (regression detection) +pytest tests/benchmarks --benchmark-only +``` + +Async tests use `pytest-asyncio` in auto mode (`pyproject.toml`). No manual marker needed for `async def test_*`. + +--- + +## How to make changes safely + +### Add a feature + +1. Read [PRD §8](PRD.md#8-decision-boundaries) — does the change pass the decision rules? +2. Read the relevant SPEC section. Does the change alter the contract? If yes, plan a SPEC update in the same commit. +3. Test-first on the **JSON backend** — fastest iteration, no external dependencies. +4. Check stability tier of the file you're touching ([docs/md/stability.md](docs/md/stability.md)). +5. Update [CHANGELOG.md](CHANGELOG.md) under `## [Unreleased]`. +6. If touching a public name, also update `jvspatial.__all__`. + +### Fix a bug + +1. Reproduce in a failing test. +2. Resolve root cause; do not paper over. +3. If the bug reveals a SPEC inaccuracy, update SPEC. +4. CHANGELOG entry under `### Fixed`. + +### Touch auth, secrets, or security + +1. Read [docs/md/security-review.md](docs/md/security-review.md) and [docs/md/security-operational-notes.md](docs/md/security-operational-notes.md). +2. Preserve constant-time comparisons. +3. Add a corresponding security-review entry for the change. + +### Touch a serverless code path + +1. Check `is_serverless_mode()` precedence (SPEC §11.1) before adding new mode-sensitive defaults. +2. Test both modes; reset the detection cache between tests. +3. If adding a Lambda-specific behavior, document the LWA interaction in [docs/md/serverless-mode.md](docs/md/serverless-mode.md). + +### Add a database backend + +1. Subclass `Database` (`jvspatial/db/database.py:48`). +2. Register via `register_database_type("name", factory_fn)` (`jvspatial/db/factory.py`). +3. Implement bulk overrides (`find_many`, `bulk_save`); the defaults are correct but slow. +4. Set `supports_transactions` capability flag. +5. Add backend-specific tests in `tests/db/`. + +--- + +## Boundaries — what NOT to do + +- Do **not** add a sync version of any I/O API. +- Do **not** broaden CORS defaults to wildcards. +- Do **not** replace `hmac.compare_digest` with `==`. +- Do **not** commit `.env` or any secrets. Use `.env.example` for templates. +- Do **not** import from underscore-prefixed modules (`_atomic`, `_path_locks`, `_cache`, `_observable`) — go through the public factory. +- Do **not** modify SPEC.md without modifying the corresponding code in the same commit (or vice versa). +- Do **not** edit `LLM-CODING-GUIDE.md` — it is preserved as a legacy reference. New documentation belongs in `docs/md/` or in PRD/SPEC/ROADMAP. +- Do **not** add features mentioning specific downstream consumers in the library or its docs. jvspatial is a foundation; consumer-specific logic lives downstream. + +--- + +## Repo geography + +``` +jvspatial/ +├── core/ # Entity hierarchy, GraphContext, events +│ ├── entities/ # Object, Node, Edge, Walker, Root +│ ├── annotations/ # @attribute system +│ └── walker_components/ # Trail, protection, queue, events +├── api/ # FastAPI integration +│ ├── auth/ # JWT, API keys, RBAC, sessions +│ ├── components/ # AppBuilder, AuthConfigurator, middleware +│ ├── decorators/ # @endpoint +│ ├── endpoints/ # Registry, factory, router +│ └── integrations/ # Webhooks, scheduler, storage service +├── db/ # Database backends + abstraction +│ ├── jsondb.py, sqlite.py, mongodb.py, dynamodb.py, postgres.py +│ ├── _atomic.py, _path_locks.py, _cache.py, _observable.py # internal +│ └── query.py, factory.py, manager.py +├── storage/ # File storage +│ ├── interfaces/ # Local, S3 +│ └── security/ # path_sanitizer, validator +├── cache/ # Memory, Redis, layered +├── serverless/ # Deferred task dispatch +├── runtime/ # Serverless detection, LWA helpers +├── observability/ # Metrics, OTEL adapter +├── logging/ # Custom levels, persisted logging +├── exceptions.py # Central exception hierarchy +├── env.py # Env helpers +├── env_adapter.py # JVSPATIAL_* allowlist + merge +└── version.py # Single source of version truth + +tests/ # Mirrors jvspatial/ layout +docs/md/ # How-to and reference docs (see docs/md/README.md) +examples/ # Runnable example scripts ``` -Tests are async (`pytest-asyncio` auto mode). Benchmarks are skipped by default — run with `pytest tests/benchmarks --benchmark-only`. +Each top-level subpackage has its own `README.md` (added in this docs pass) — read that before diving into source. + +--- + +## Slash commands and skills relevant in this repo + +- `pytest` and `pytest --benchmark-only` are the canonical test entry points. +- `pre-commit run --all-files` runs the full linter/formatter suite (black, isort, flake8, mypy, detect-secrets). +- For larger refactors, follow the [docs/md/contributing.md](docs/md/contributing.md) workflow; for security-sensitive changes, the workflow in [docs/md/security-review.md](docs/md/security-review.md). + +--- + +## Pointers to authoritative sources + +When in doubt, the source-of-truth order is: + +1. Code (always) +2. [SPEC.md](SPEC.md) (technical contract; should match code) +3. [PRD.md](PRD.md) (product context) +4. [docs/md/](docs/md/) (how-to) +5. [CHANGELOG.md](CHANGELOG.md) (release notes) +6. [LLM-CODING-GUIDE.md](LLM-CODING-GUIDE.md) (legacy; informational only, not contractual) --- -## Editor / agent-tool specific notes +Update this file in the same commit as any change that alters an invariant or boundary above. -This section is the only place where agent-tool-specific guidance lives. Keep it short; cross-link to CLAUDE.md for everything else. +## Editor and agent-tool notes -- **Cursor / Continue**: `.cursor/` directory is gitignored except for committed rules. There are no committed `.cursorrules`; this file plus CLAUDE.md is the full agent contract. -- **Aider**: `--read CLAUDE.md` to load the canonical guide on session start. -- **Codex / OpenHands**: this file is loaded as agent instructions; read CLAUDE.md for full details. +- **Cursor / Continue**: There are no committed `.cursorrules`; this file is the full agent contract. +- **Aider**: Use `--read AGENTS.md` on session start. +- **Codex / OpenHands / Claude Code**: Read this file as the canonical guide. -Any agent-specific quirk worth recording goes here, with a one-line rationale. If a quirk grows beyond a line, promote it into CLAUDE.md and reference back. +Keep tool-specific notes brief; promote a rule that applies to all agents into the main guide above. diff --git a/AUDIT.md b/AUDIT.md index 5b79a98..235bc39 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1,6 +1,6 @@ # jvspatial — Foundation Audit -> **Purpose**: First retrospective audit of the jvspatial foundation against the contracts now codified in [SPEC.md](SPEC.md), [PRD.md](PRD.md), and [CLAUDE.md](CLAUDE.md). Findings are the backlog for the next phase of hardening. +> **Purpose**: First retrospective audit of the jvspatial foundation against the contracts now codified in [SPEC.md](SPEC.md), [PRD.md](PRD.md), and [AGENTS.md](AGENTS.md). Findings are the backlog for the next phase of hardening. > > **Method**: Five parallel reviewers, one per dimension: async contract, security boundaries, database adapter parity, walker/identity invariants, serverless/config/stability. Each returned a severity-tagged finding table with file:line citations. > @@ -21,7 +21,7 @@ Top themes: 3. **Async contract has real holes, mostly in JsonDB.** Five CRITs: webhook walker `enhanced_init` is `async def __init__` (Python ignores; coroutine leaks every construction), webhook `endpoint_func(**kwargs)` is unawaited, storage `delete_file` unawaited, two `Path.write_text` calls inside `async def` graph exporters. Plus a string of `path.exists()` / `path.glob()` sync stats inside JsonDB `async` methods that block the event loop. 4. **Storage path traversal in `LocalFileInterface` versioning methods.** Primary save/read/delete go through `PathSanitizer`; the versioning subpaths (`create_version`, `get_version`, `list_versions`, `delete_version`, `get_latest_version`) compute `root_dir / f"{file_path}.versions"` without sanitization — user-controlled `file_path` like `../../etc/passwd` escapes the storage root. 5. **Webhook HMAC verification is broken.** A 7-character slice bug in `webhook_auth.utils.verify_signature` makes `hmac.compare_digest` always compare a 64-char hex digest to a 57-char prefix → always False. Webhook signature auth currently rejects every request. -6. **SHA-256 fallback uses `==`, not `compare_digest`.** `AuthenticationService._verify_refresh_token` falls back to `hashlib.sha256(token) == hashed` when bcrypt/argon2 fail to import; this path covers refresh tokens AND password-reset tokens. CLAUDE.md §2 non-negotiable. +6. **SHA-256 fallback uses `==`, not `compare_digest`.** `AuthenticationService._verify_refresh_token` falls back to `hashlib.sha256(token) == hashed` when bcrypt/argon2 fail to import; this path covers refresh tokens AND password-reset tokens. AGENTS.md §2 non-negotiable. 7. **DynamoDB throttle-retry is partial.** Only `save`/`get`/`delete` are wrapped. `find` / `count` / `batch_get` / `batch_write` surface `ProvisionedThroughputExceededException` directly to callers, even though the adapter claims throttle retry (SPEC §4.3). 8. **Env-var allowlist is not enforced.** SPEC §10.2 promises "Unknown `JVSPATIAL_*` keys are rejected at startup to catch typos." `env_adapter.py` only *reads* enumerated keys; it never *scans* the environment for stray `JVSPATIAL_*` and rejects. Plus three divergent `parse_bool` implementations across `env.py` / `env_adapter.py` / `runtime/serverless.py`. @@ -33,7 +33,7 @@ These are listed below with file:line citations and one-line fixes. The audit wa - **Sections** are by dimension (async, security, database, walker, serverless). - **Within a section**, findings sorted by severity then file. -- **`Cite`** column references SPEC §, CLAUDE.md §, or ROADMAP §. +- **`Cite`** column references SPEC §, AGENTS.md §, or ROADMAP §. - **`Fix`** is the smallest change that closes the gap, not necessarily the best long-term fix. When the same root cause shows up in multiple dimensions, it is listed once (under the most specific dimension) and cross-referenced. @@ -62,7 +62,7 @@ The most important class of findings. SPEC §1.2: `__entity_name__` is per-subcl ## 2. Walker Protection Gaps -SPEC §6.3 / CLAUDE.md §9. Promised limits are partially unenforced. +SPEC §6.3 / AGENTS.md §9. Promised limits are partially unenforced. | # | Sev | File:Line | Problem | Fix | Cite | |---|---|---|---|---|---| @@ -81,7 +81,7 @@ SPEC §6.3 / CLAUDE.md §9. Promised limits are partially unenforced. ## 3. Async Contract Violations -CLAUDE.md §1 / SPEC §3. Five real bugs, plus a long tail of `path.glob`/`path.exists` sync stats blocking the event loop in JsonDB. +AGENTS.md §1 / SPEC §3. Five real bugs, plus a long tail of `path.glob`/`path.exists` sync stats blocking the event loop in JsonDB. | # | Sev | File:Line | Problem | Fix | Cite | |---|---|---|---|---|---| @@ -103,25 +103,25 @@ CLAUDE.md §1 / SPEC §3. Five real bugs, plus a long tail of `path.glob`/`path. ## 4. Security Boundaries -SPEC §15 / CLAUDE.md §2. +SPEC §15 / AGENTS.md §2. | # | Sev | File:Line | Problem | Fix | Cite | |---|---|---|---|---|---| -| 4.1 | CRIT | `api/auth/service.py:367` | `hashlib.sha256(token).hexdigest() == hashed` — refresh-token / password-reset-token hash compared with `==`. | `hmac.compare_digest(...)`. | SPEC §15.2, CLAUDE §2 | +| 4.1 | CRIT | `api/auth/service.py:367` | `hashlib.sha256(token).hexdigest() == hashed` — refresh-token / password-reset-token hash compared with `==`. | `hmac.compare_digest(...)`. | SPEC §15.2, AGENTS §2 | | 4.2 | CRIT | `storage/interfaces/local.py:207, 220, 252, 260, 288, 327, 332, 333, 356` | All file-versioning methods compute `root_dir / f"{file_path}.versions"` without `PathSanitizer`; user-controlled `file_path` escapes the storage root. | Sanitize `file_path` through `_get_full_path()` before building versioned paths; resolve and `.relative_to(self.root_dir)` the result. | SPEC §15.1 | | 4.3 | CRIT | `api/integrations/webhooks/utils.py:176` | `hmac.compare_digest(signature, expected_signature[len(prefix):])` — `expected_signature` is the bare hex digest; slicing 7 chars produces 57-char string vs 64-char signature → always False. Webhook HMAC always rejects. | Drop the slice: `compare_digest(signature, expected_signature)`. | SPEC §15.2 | -| 4.4 | HIGH | `api/auth/api_key_service.py:30` | `self.context = context or get_default_context()` — auth state can land on non-prime DB when a caller forgets to pass `context`. | `context or GraphContext(database=get_prime_database())`. | SPEC §9, CLAUDE §1 | +| 4.4 | HIGH | `api/auth/api_key_service.py:30` | `self.context = context or get_default_context()` — auth state can land on non-prime DB when a caller forgets to pass `context`. | `context or GraphContext(database=get_prime_database())`. | SPEC §9, AGENTS §1 | | 4.5 | HIGH | `api/auth/api_key_service.py:213-246` + `api/integrations/webhooks/webhook_auth.py:19-21, 180-183` | `revoke_key` flips `is_active=False` in DB but does not invalidate the 300s in-memory `_API_KEY_CACHE`. Revoked key authenticates for up to 5 minutes after revocation. | Add `webhook_auth.invalidate_cache(...)` hook called from `revoke_key`; or re-check `is_active` on cache hit. | SPEC §15.2 | | 4.6 | HIGH | `api/components/auth_configurator.py:175-391` | `/auth/register`, `/auth/login`, `/auth/forgot-password`, `/auth/reset-password`, `/auth/change-password` registered without endpoint rate-limit configs; fallback is global `default_limit=60/60s` if rate limiting is enabled at all. | Hard-code rate-limit configs during `_register_auth_endpoints`; document rate-limit middleware as required when auth enabled. | SPEC §15 | -| 4.7 | HIGH | `api/integrations/webhooks/webhook_auth.py:19, 147-183` | `_API_KEY_CACHE` mutated across `await` without lock; eviction at size cap races with reads (`KeyError`). | Wrap in `asyncio.Lock`, or use bounded LRU with single-lock guard. | CLAUDE.md "race conditions in auth state" | -| 4.8 | HIGH | `api/auth/enhanced.py:235-389` | `SessionManager._sessions` / `_user_sessions` mutated across `await` without lock; concurrent logout-vs-login → `RuntimeError: dictionary changed size during iteration`; `max_sessions_per_user` enforcement is racy. | `asyncio.Lock` around session create/invalidate/cleanup. | CLAUDE.md races | +| 4.7 | HIGH | `api/integrations/webhooks/webhook_auth.py:19, 147-183` | `_API_KEY_CACHE` mutated across `await` without lock; eviction at size cap races with reads (`KeyError`). | Wrap in `asyncio.Lock`, or use bounded LRU with single-lock guard. | AGENTS.md "race conditions in auth state" | +| 4.8 | HIGH | `api/auth/enhanced.py:235-389` | `SessionManager._sessions` / `_user_sessions` mutated across `await` without lock; concurrent logout-vs-login → `RuntimeError: dictionary changed size during iteration`; `max_sessions_per_user` enforcement is racy. | `asyncio.Lock` around session create/invalidate/cleanup. | AGENTS.md races | | 4.9 | HIGH | `storage/interfaces/local.py:657-659` | `get_metadata` MIME detection passes `content=b""` — falls back to extension-based `mimetypes.guess_type`; mislabels served `Content-Type`. | Read first 4 KiB of file, pass as content; or store validated MIME in sidecar at save. | SPEC §15.1 | | 4.10 | HIGH | `api/components/error_handler.py:763, 769-770` | If operator sets `JVSPATIAL_EXPOSE_ERROR_DETAILS=true` in production, raw exception messages leak in 500 responses; safe default exists but no guard. | Refuse to honor the flag when `get_environment_mode() == "production"`. | SPEC §15.5 | | 4.11 | MED | `api/auth/service.py:427` | Debug log discloses JWT secret length. | Log `secret_configured=bool(secret)` only. | SPEC §15.5 | | 4.12 | MED | `api/middleware/manager.py:194-197` + `api/config_groups.py:66-75` | `CORSConfig` accepts wildcard origins; no startup validator. SPEC says wildcards must trigger startup warning. | Add validator on `CORSConfig` to warn on `"*"`; fail loudly if `allow_credentials=True` + wildcard. | SPEC §15.4 | | 4.13 | MED | `api/auth/service.py:1052-1059` | `validate_token` warning logs the DB `base_path` — discloses internal filesystem path. | Log `db_type=type(database).__name__`. | SPEC §15.5 | | 4.14 | MED | `api/auth/service.py:1038-1049` | DB-error fallback in `validate_token` trusts JWT-payload roles when DB lookup fails — fails open on DB outage, within JWT-expiry window. | Fail closed: return `None` on DB error. | SPEC §9 | -| 4.15 | MED | `api/auth/service.py:469-531` | `_blacklist_cache` dict mutated across `await` without lock; unbounded growth (per-entry TTL only). | Add `asyncio.Lock` + size cap with LRU eviction. | CLAUDE.md races | +| 4.15 | MED | `api/auth/service.py:469-531` | `_blacklist_cache` dict mutated across `await` without lock; unbounded growth (per-entry TTL only). | Add `asyncio.Lock` + size cap with LRU eviction. | AGENTS.md races | | 4.16 | LOW | `api/deferred_invoke_route.py:28-37` | Empty `JVSPATIAL_DEFERRED_INVOKE_SECRET` returns `True` from `_deferred_invoke_secret_ok` — misconfigured deployment exposes internal endpoint. | Treat empty secret as "deny all". | SPEC §15.2 | | 4.17 | LOW | `api/auth/service.py:331, 243-244` | `JVSPATIAL_AUTH_STRICT_HASHING` defaults True for passwords, False for tokens — asymmetry surprising. | Apply strict to both; document rationale. | SPEC §15.5 | | 4.18 | LOW | `storage/interfaces/local.py:194-196` | Windows reserved names (CON, AUX, NUL) pass `SAFE_FILENAME_PATTERN`. | Add Windows-reserved-name check. | SPEC §15.1 | @@ -162,9 +162,9 @@ SPEC §4-5 / ROADMAP §2.2, §2.4. | 6.2 | HIGH | `core/entities/edge.py:66-123` | Same: `Edge.__init_subclass__` skips `super()`. | Same fix. | SPEC §2.5 | | 6.3 | HIGH | `core/entities/walker.py:366-396` | Same: `Walker.__init_subclass__` skips `super()`. | Same fix. | SPEC §2.5 | | 6.4 | HIGH | `core/entities/root.py:19` | `_lock = asyncio.Lock()` is `ClassVar` — single global lock across all GraphContexts and event loops; "different loop" errors under per-test loop fixtures; SPEC §1.3 implies per-context. | Lock per `GraphContext` (e.g. lazy-init dict keyed by `id(context)`). | SPEC §1.3, §7.2 | -| 6.5 | HIGH | `core/entities/root.py:100` | `object.__setattr__(self, "id", "n.Root.root")` — library bypasses its own `protected=True`. | `_unsafe_set_id` helper, or check/clear `_initializing` and route through normal setter. | CLAUDE §5 | -| 6.6 | HIGH | `core/context.py:761, 812, 818, 1229` | Six `object.__setattr__` call sites bypass protected-attribute enforcement (id, edge_ids, atomic_increment). | Route through `AttributeMixin.__setattr__` with explicit override flag. | CLAUDE §5 | -| 6.7 | MED | `core/mixins/deferred_save.py:119+` | No runtime check of MRO order — wrong order silently disables batching (CLAUDE.md warns but library doesn't detect). | In `__init_subclass__`, assert mixin precedes persistable base; warn or raise. | CLAUDE §6 | +| 6.5 | HIGH | `core/entities/root.py:100` | `object.__setattr__(self, "id", "n.Root.root")` — library bypasses its own `protected=True`. | `_unsafe_set_id` helper, or check/clear `_initializing` and route through normal setter. | AGENTS §5 | +| 6.6 | HIGH | `core/context.py:761, 812, 818, 1229` | Six `object.__setattr__` call sites bypass protected-attribute enforcement (id, edge_ids, atomic_increment). | Route through `AttributeMixin.__setattr__` with explicit override flag. | AGENTS §5 | +| 6.7 | MED | `core/mixins/deferred_save.py:119+` | No runtime check of MRO order — wrong order silently disables batching (AGENTS.md warns but library doesn't detect). | In `__init_subclass__`, assert mixin precedes persistable base; warn or raise. | AGENTS §6 | | 6.8 | MED | `core/entities/object.py:112-140` | `__setattr__` allows ANY `name.startswith("_")` — callers can attach arbitrary `_foo` attributes, bypassing schema validation (SPEC promises rejection). | Restrict private bypass to declared `__private_attributes__` + `_initializing`. | SPEC §2.1 | | 6.9 | MED | `core/events.py:36, 119`, `core/context.py:1818-1843` | `EventBus._lock` and `event_bus` module global bound to import-time loop; tests with new loops fail. Also `get_default_context()` has check-and-set race. | Lazy-init locks per first-async-use; use `ContextVar.get` + token for default context init. | SPEC §6.x, §7.1 | | 6.10 | MED | `core/events.py:38-45` | `register_entity` on `walker.spawn()` has no symmetric `unregister_entity` — events keep firing to done walkers; weakref GC only. | Call `event_bus.unregister_entity(self.id)` in `disengage()` and end-of-`spawn`. | SPEC §6.x | @@ -174,7 +174,7 @@ SPEC §4-5 / ROADMAP §2.2, §2.4. ## 7. Serverless / Config / Stability Discipline -SPEC §10-11, §18 / CLAUDE.md §4, §7, §8. +SPEC §10-11, §18 / AGENTS.md §4, §7, §8. | # | Sev | File:Line | Problem | Fix | Cite | |---|---|---|---|---|---| @@ -182,12 +182,12 @@ SPEC §10-11, §18 / CLAUDE.md §4, §7, §8. | 7.2 | HIGH | `env_adapter.py:11` vs `env.py:36-43` vs `runtime/serverless.py:11` | Three divergent `parse_bool` implementations. `JVSPATIAL_DEBUG=on` parses different ways in different code paths. | Consolidate on `env.parse_bool`; have others import. | SPEC §10.2 | | 7.3 | MED | `env_adapter.py:11`, `runtime/serverless.py:11` | `_parse_bool` non-strict; `JVSPATIAL_DEBUG=garbage` silently maps to False with no validation error. Hides typos. | Raise `ValueError` on unrecognized non-empty. | SPEC §10.2 | | 7.4 | MED | `serverless/deferred_invoke.py:52-73` | `normalize_deferred_envelope` covers flat Lambda invoke + SQS scheduler shapes only. Direct Lambda SQS-batch trigger (`{"Records": [...]}`) raises. | Accept `Records[...]` and dispatch per-record, OR document the unwrap requirement. | SPEC §11.3 | -| 7.5 | MED | `.env.example:94-106` vs `api/config_groups.py:66-81` | `.env.example` documents `JVSPATIAL_CORS_ORIGINS=*` with "Default: *" — actual default is localhost whitelist. Following docs degrades security. | Update `.env.example`; add production-only note. | CLAUDE §8 | +| 7.5 | MED | `.env.example:94-106` vs `api/config_groups.py:66-81` | `.env.example` documents `JVSPATIAL_CORS_ORIGINS=*` with "Default: *" — actual default is localhost whitelist. Following docs degrades security. | Update `.env.example`; add production-only note. | AGENTS §8 | | 7.6 | MED | `runtime/lwa.py:107-110` (called from `server.py:137`) | LWA env defaults applied inside `Server.__init__`; LWA reads them before Python starts. The docstring acknowledges this; the practical effect is zero for the actual LWA bootstrap. | Downgrade to operator warning, or remove and document IaC-only. | SPEC §11.4 | | 7.7 | MED | `db/transaction.py:243-249` | `JsonDBTransaction(best_effort=True)` imports private `_emit_once` from `jvspatial.utils.stability` — internal symbol crossed module boundary. | Expose public `emit_experimental_once(...)`. | SPEC §18 | | 7.8 | MED | `serverless/deferred_invoke.py:1-88` | Handlers registered post-import → race: request arriving before user modules import returns 404 `UnknownDeferredTaskError`. Not silently dropped (good); but no startup-readiness log. | Log registered handlers on startup, or log debug on empty-registry first-dispatch. | SPEC §11.3 | | 7.9 | LOW | `env_adapter.py:39-49` | `deep_merge` silently skips `None` in override; `Server(host=None, ...)` does NOT override env. Surprising. | Document; or sentinel-based override. | SPEC §10.2 | -| 7.10 | LOW | `api/auth/service.py:164-170` | `AuthenticationService` captures `is_serverless_mode()` and `_bcrypt_rounds` at construction; per-test mode flips require service rebuild + `reset_serverless_mode_cache()`. | Note in CLAUDE.md §4 (test guidance). | CLAUDE §4 | +| 7.10 | LOW | `api/auth/service.py:164-170` | `AuthenticationService` captures `is_serverless_mode()` and `_bcrypt_rounds` at construction; per-test mode flips require service rebuild + `reset_serverless_mode_cache()`. | Note in AGENTS.md §4 (test guidance). | AGENTS §4 | | 7.11 | LOW | `runtime/lwa.py:18-23` | `_deferred_invoke_pass_through_path` reads `JVSPATIAL_API_PREFIX` directly, bypassing `resolve_api_prefix()`. Two divergent reads. | Use `resolve_api_prefix()`. | SPEC §10.4 | | 7.12 | LOW | `api/components/app_builder.py:58-65` | `JVSPATIAL_DOCS_DISABLED` parses with ad-hoc inline set `{"1","true","yes","on"}` — fourth divergent bool parser. | Use `env.parse_bool`. | SPEC §10.5 | | 7.13 | LOW | `api/middleware/manager.py:54-59` | `_DOCS_PATH_PREFIXES = ("/docs", "/redoc", "/openapi.json")` hardcoded — customizing `docs_url` breaks Swagger UI via strict CSP. | Derive from `config.docs_url`/`redoc_url`/`openapi_url`. | SPEC §10.5 | diff --git a/CHANGELOG.md b/CHANGELOG.md index 18a4808..bf964f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- PostgreSQL `$exists` now treats JSON `null` like the in-memory query engine, fixing queries for absent or null nested values. +- File-backed SQLite closes the old `aiosqlite` connection when rebinding across event loops, preventing a leaked worker thread from keeping the process alive. +- Graph transactions isolate their request identity map and invalidate touched parent cache entries after commit or rollback. Index setup is cached per database instance so a second store receives its own indexes. +- Tests now restore mocked walker metadata, bind their own graph context, and enter the test client's lifespan before traversing Root, removing order-dependent suite failures. + +## [0.1.0] - 2026-09-27 + +### Security + +- OAuth RS256 signing keys can be encrypted at rest with `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY`. Existing plaintext keys rewrap on first load; encrypted keys fail closed when the key is missing or wrong. Production OAuth hosts must configure and retain the key. + +- In-memory `$regex` queries now have a per-candidate execution timeout, preventing short catastrophic-backtracking patterns from monopolizing the server. Raw FastAPI route auth detection now requires an enforcing security dependency; dependency names no longer bypass framework auth. +- Webhook idempotency now atomically reserves a key before invoking the handler and persists the response for replay. In-flight or uncertain outcomes return 409; persistence failures return 503 and require reconciliation before retry. +- JWT validation now rejects user lookup failures and uses current database roles and permissions. Logout invalidates the access token and its bound refresh token; token revocation failures are surfaced. Public registration never grants admin access. Auth entry points have per-IP rate limits, generic account-state errors, and a 12-character password minimum. +- Signed GET webhooks require HMAC. Webhook routes default to POST, API-key scopes match path segments, expired cached keys are rejected, and idempotency lookup errors fail closed. Webhook handlers finish within the request; durable asynchronous work must use an external queue. +- JsonDB record paths and local storage version IDs are confined to their roots. SQL index field paths are validated and raw `where=` predicates are rejected. Graph deletion, walker errors, DynamoDB bulk writes, and deferred operations now report failures instead of success. +- **BREAKING:** Direct file reads require authentication by default when auth is enabled. Set `JVSPATIAL_FILES_PUBLIC_READ=true` only when intentional. +- **BREAKING:** `JVSPATIAL_ENVIRONMENT=production` unpublishes `/docs`, `/redoc`, and `/openapi.json` by default. Set `JVSPATIAL_DOCS_DISABLED=false` for an explicit override. +- **BREAKING:** Deferred invoke requires a secret even on loopback. Set `JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK=true` only for Lambda Web Adapter self-invoke. +- **BREAKING:** The in-process scheduler requires a running server event loop and runs coroutine jobs on that loop. Jobs started outside an event loop must use a configured external scheduler. +- Hosts with their own trusted auth limiter or test suites can explicitly set `rate_limit.auth_entrypoint_rate_limit_enabled=False`; the 5/60s cap remains on by default. Declared private helper methods can be replaced on entity instances, while undeclared underscore attributes remain rejected. +- Authenticated mounted ASGI apps and raw FastAPI routes without jvspatial endpoint metadata remain reachable. Metadata lookup errors still deny access, and `/status`, `/logs`, and `/graph` retain enforced admin roles. + +### Changed + +- `AGENTS.md` is now the canonical agent guide. Its former `CLAUDE.md` content has been consolidated there; `CLAUDE.md` is removed. +- The deprecated `generate_id_async` alias remains available in 0.1.0; its removal target is 0.2.0. + ## [0.0.22] - 2026-09-26 ### Added @@ -598,7 +628,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Deprecated -- `jvspatial.core.utils.generate_id_async` — deprecated alias for `generate_id`. ID generation is pure computation (SPEC §3.2); the async signature was a vestige. Scheduled for removal in 0.1.0. (Audit §3.11.) +- `jvspatial.core.utils.generate_id_async` — deprecated alias for `generate_id`. ID generation is pure computation (SPEC §3.2); the async signature was a vestige. Removal target updated to 0.2.0 in the 0.1.0 release. (Audit §3.11.) ### Removed diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 924254c..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,246 +0,0 @@ -# CLAUDE.md — Agent Guide for jvspatial - -> This file is read automatically by Claude Code at the start of every session in this repo. It is the **canonical entry point** for any AI agent maintaining `jvspatial`. Keep it short, current, and actionable. - ---- - -## What jvspatial is - -`jvspatial` is an **async-first, serverless-compatible, object-spatial Python library** for graph-based persistence and business-logic layers. It layers an entity-centric graph model (Object → Node / Edge / Walker / Root) over four database backends (JSON, SQLite, MongoDB, DynamoDB), and ships a FastAPI integration with auth, file storage, observability, and serverless ergonomics. - -For full positioning and non-goals, read [PRD.md](PRD.md). -For the technical contract, read [SPEC.md](SPEC.md). - ---- - -## Where to look first - -When given a task, resolve context in this order: - -1. **[PRD.md](PRD.md)** — what's the product context? Is this in scope? -2. **[SPEC.md](SPEC.md)** — what does the library currently guarantee about the area you're touching? Every claim cites a `file:line`. -3. **[docs/md/README.md](docs/md/README.md)** — index of how-to docs. Find the relevant one before reading source. -4. **[ROADMAP.md](ROADMAP.md)** — is this area under active hardening? Known gap? Out of scope? -5. **[CHANGELOG.md](CHANGELOG.md)** — recent changes that may affect your work. -6. **Source code** — last, not first. The map of subpackage READMEs (`jvspatial/*/README.md`) shortens this hop. - -If a doc disagrees with code, the doc is wrong. File an issue and trust the code — but cite the discrepancy. - ---- - -## Non-negotiable invariants - -These are properties the library guarantees. Breaking them is a regression even if tests pass. - -### 1. Async-only I/O - -Every database call, network call, and file-system call is `async`. There are **no sync wrappers** in the library. Sync is for pure computation only. - -- ❌ `entity.save()` (missing `await`) -- ✅ `await entity.save()` - -If you find yourself wanting a sync wrapper, you are solving the wrong problem. - -### 2. Constant-time secret comparison - -Any comparison of a secret, key, token, or password hash uses `hmac.compare_digest`. Affected paths (SPEC §15.2): -- API key verification, refresh token comparison, password reset token, webhook HMAC, deferred-invoke secret, bcrypt legacy fallback. - -Never replace `hmac.compare_digest` with `==` for performance or readability. - -### 3. Entity name override (`__entity_name__`) - -Persisted entity discriminator is `cls.__dict__.get("__entity_name__") or cls.__name__` (SPEC §1.2). Resolution is **per-subclass**, not inherited. Code that constructs IDs or looks up subclasses must go through `generate_id()` and `find_subclass_by_name()` — never assemble IDs from `cls.__name__` directly. - -### 4. Serverless detection precedence - -`is_serverless_mode(config=None)` resolves: explicit config → current Server config → `SERVERLESS_MODE` env → auto-detection (SPEC §11.1). Do not bypass with custom env reads. Do not memoize results across tests — call `reset_serverless_mode_cache()` between cases. - -### 5. Protected attribute validation - -`Object.__setattr__` validates field names against the class hierarchy. Setting an undeclared attribute on an `Object` is rejected. Never use `object.__setattr__(self, ...)` to bypass; if you need a new field, declare it. - -### 6. Deferred-save MRO - -`class MyEntity(DeferredSaveMixin, Node)` — mixin must come **before** the base. Wrong order silently disables batching. Tests should assert MRO-sensitive behavior. - -### 7. Stability tier discipline - -Symbols in `jvspatial.__all__` are public — breaking changes require a deprecation cycle (`docs/md/stability.md`). Underscore-prefixed modules are internal — callers should not import directly. Promoting an internal helper to public is a product decision; update PRD §7 and docs/md/stability.md together. - -### 8. CORS / CSP / docs defaults - -CORS does **not** default to wildcard. CSP is strict on app routes, relaxed only on `/docs`, `/redoc`, `/openapi.json`. `JVSPATIAL_DOCS_DISABLED` is the production posture. Do not weaken these defaults without a security review entry. - -### 9. Walker protection - -`max_steps=10000`, `max_visits_per_node=100`, `max_execution_time=300s`, `max_queue_size=1000` are defaults that prevent DOS. Disabling protection is allowed locally; never disable globally or in code that touches user input. - ---- - -## Common gotchas - -| Symptom | Cause | Fix | -|---|---|---| -| `TypeError: object NoneType can't be used in 'await' expression` | Forgot `await` on async DB call | Add `await` | -| Custom mixin doesn't batch saves | `DeferredSaveMixin` placed after base in MRO | Mixin first: `class X(DeferredSaveMixin, Node)` | -| Subclass lookup returns wrong class | Two unrelated classes share `__name__` | Set `__entity_name__: ClassVar[Optional[str]] = "Distinct"` on one | -| Tests pass locally, fail under serverless mode | Forgot to test both modes | Use `reset_serverless_mode_cache()` between tests | -| 401 with valid token | Auth state on wrong database | Auth is **always** on prime DB; do not relocate | -| Slow query never logged | `slow_query_ms` not configured | Set in `create_database(observe=True, slow_query_ms=N)` | -| Walker visits the same node forever | Protection disabled | Re-enable `protection_enabled=True` | -| `JVSPATIAL_FOO` env var ignored | Not in allowlist | Add to `jvspatial/env_adapter.py` allowlist | -| SQLite query falls back to in-memory filter | Operator not yet pushed down by `SQLiteTranslator` | Check `jvspatial/db/_sqlite_translate.py` for supported operators | - ---- - -## Graph modeling convention - -Convention for any application built on jvspatial. Not enforced by the library, but required by review and assumed by downstream tooling. - -1. **Every `Node` must be reachable from `Root`** — directly or through intermediate nodes. An orphaned node is a bug. If a record does not belong in the graph, it must not be a `Node`. -2. **Applications declare an application-root node** — e.g. `class App(Node)`, `class MyApp(Node)`. Create exactly one instance, connect it to `Root`, and treat it as the entry point for all application state. -3. **All application nodes extend from the app-root** — directly or under an appropriate branch / category node beneath it (e.g. `App -> Users -> User`, `App -> Catalog -> Product`). Do not connect application nodes directly to `Root`; `Root` belongs to the library, the app-root belongs to the application. -4. **Use `Object` (not `Node`) for record-style data that does not benefit from graph membership** — audit/change-event records, denormalized snapshots, ledger entries, raw inbound payloads. `ChangeEvent` **must be an `Object`**, not a `Node`. Rule of thumb: if nothing will ever traverse to or from it, it is not a `Node`. - -When reviewing or generating code, reject `Node` subclasses that are never connected, and reject `Object` subclasses that are used as relationship endpoints. - ---- - -## Run the dev loop - -```bash -# install -pip install -e '.[dev,test]' -pre-commit install - -# fast feedback -pytest -q # unit + integration (skips benchmarks) -pre-commit run --all-files # lint / format / type-check - -# full quality bar (run before opening PR) -pytest --cov=jvspatial --cov-report=term-missing - -# benchmarks (regression detection) -pytest tests/benchmarks --benchmark-only -``` - -Async tests use `pytest-asyncio` in auto mode (`pyproject.toml`). No manual marker needed for `async def test_*`. - ---- - -## How to make changes safely - -### Add a feature - -1. Read [PRD §8](PRD.md#8-decision-boundaries) — does the change pass the decision rules? -2. Read the relevant SPEC section. Does the change alter the contract? If yes, plan a SPEC update in the same commit. -3. Test-first on the **JSON backend** — fastest iteration, no external dependencies. -4. Check stability tier of the file you're touching ([docs/md/stability.md](docs/md/stability.md)). -5. Update [CHANGELOG.md](CHANGELOG.md) under `## [Unreleased]`. -6. If touching a public name, also update `jvspatial.__all__`. - -### Fix a bug - -1. Reproduce in a failing test. -2. Resolve root cause; do not paper over. -3. If the bug reveals a SPEC inaccuracy, update SPEC. -4. CHANGELOG entry under `### Fixed`. - -### Touch auth, secrets, or security - -1. Read [docs/md/security-review.md](docs/md/security-review.md) and [docs/md/security-operational-notes.md](docs/md/security-operational-notes.md). -2. Preserve constant-time comparisons. -3. Add a corresponding security-review entry for the change. - -### Touch a serverless code path - -1. Check `is_serverless_mode()` precedence (SPEC §11.1) before adding new mode-sensitive defaults. -2. Test both modes; reset the detection cache between tests. -3. If adding a Lambda-specific behavior, document the LWA interaction in [docs/md/serverless-mode.md](docs/md/serverless-mode.md). - -### Add a database backend - -1. Subclass `Database` (`jvspatial/db/database.py:48`). -2. Register via `register_database_type("name", factory_fn)` (`jvspatial/db/factory.py`). -3. Implement bulk overrides (`find_many`, `bulk_save`); the defaults are correct but slow. -4. Set `supports_transactions` capability flag. -5. Add backend-specific tests in `tests/db/`. - ---- - -## Boundaries — what NOT to do - -- Do **not** add a sync version of any I/O API. -- Do **not** broaden CORS defaults to wildcards. -- Do **not** replace `hmac.compare_digest` with `==`. -- Do **not** commit `.env` or any secrets. Use `.env.example` for templates. -- Do **not** import from underscore-prefixed modules (`_atomic`, `_path_locks`, `_cache`, `_observable`) — go through the public factory. -- Do **not** modify SPEC.md without modifying the corresponding code in the same commit (or vice versa). -- Do **not** edit `LLM-CODING-GUIDE.md` — it is preserved as a legacy reference. New documentation belongs in `docs/md/` or in PRD/SPEC/ROADMAP. -- Do **not** add features mentioning specific downstream consumers in the library or its docs. jvspatial is a foundation; consumer-specific logic lives downstream. - ---- - -## Repo geography - -``` -jvspatial/ -├── core/ # Entity hierarchy, GraphContext, events -│ ├── entities/ # Object, Node, Edge, Walker, Root -│ ├── annotations/ # @attribute system -│ └── walker_components/ # Trail, protection, queue, events -├── api/ # FastAPI integration -│ ├── auth/ # JWT, API keys, RBAC, sessions -│ ├── components/ # AppBuilder, AuthConfigurator, middleware -│ ├── decorators/ # @endpoint -│ ├── endpoints/ # Registry, factory, router -│ └── integrations/ # Webhooks, scheduler, storage service -├── db/ # Database backends + abstraction -│ ├── jsondb.py, sqlite.py, mongodb.py, dynamodb.py -│ ├── _atomic.py, _path_locks.py, _cache.py, _observable.py # internal -│ └── query.py, factory.py, manager.py -├── storage/ # File storage -│ ├── interfaces/ # Local, S3 -│ └── security/ # path_sanitizer, validator -├── cache/ # Memory, Redis, layered -├── serverless/ # Deferred task dispatch -├── runtime/ # Serverless detection, LWA helpers -├── observability/ # Metrics, OTEL adapter -├── logging/ # Custom levels, persisted logging -├── exceptions.py # Central exception hierarchy -├── env.py # Env helpers -├── env_adapter.py # JVSPATIAL_* allowlist + merge -└── version.py # Single source of version truth - -tests/ # Mirrors jvspatial/ layout -docs/md/ # How-to and reference docs (see docs/md/README.md) -examples/ # Runnable example scripts -``` - -Each top-level subpackage has its own `README.md` (added in this docs pass) — read that before diving into source. - ---- - -## Slash commands and skills relevant in this repo - -- `pytest` and `pytest --benchmark-only` are the canonical test entry points. -- `pre-commit run --all-files` runs the full linter/formatter suite (black, isort, flake8, mypy, detect-secrets). -- For larger refactors, follow the [docs/md/contributing.md](docs/md/contributing.md) workflow; for security-sensitive changes, the workflow in [docs/md/security-review.md](docs/md/security-review.md). - ---- - -## Pointers to authoritative sources - -When in doubt, the source-of-truth order is: - -1. Code (always) -2. [SPEC.md](SPEC.md) (technical contract; should match code) -3. [PRD.md](PRD.md) (product context) -4. [docs/md/](docs/md/) (how-to) -5. [CHANGELOG.md](CHANGELOG.md) (release notes) -6. [LLM-CODING-GUIDE.md](LLM-CODING-GUIDE.md) (legacy; informational only, not contractual) - ---- - -**Last updated**: Same commit as the PRD/SPEC/ROADMAP docs pass. Update this file in the same commit as any change that alters an invariant or boundary above. diff --git a/PRD.md b/PRD.md index c820f62..fcc54b6 100644 --- a/PRD.md +++ b/PRD.md @@ -126,7 +126,7 @@ Explicit out-of-scope statements. These represent decisions, not gaps. - **A caller building a new graph-backed service can get to a working CRUD endpoint in under 30 minutes**, following [docs/md/quick-start-guide.md](docs/md/quick-start-guide.md) and one of the [examples/api/](examples/api/) reference implementations, without reading any source code. - **A caller can switch backends (JSON → SQLite → MongoDB → DynamoDB) without rewriting queries or entities** — only configuration changes. - **A caller deploying to Lambda does not need to write Lambda-specific code**, beyond the entry adapter for their function. Defaults adapt; deferred work has a single registration point. -- **An AI agent maintaining the library can locate the right code path from a single CLAUDE.md read**, without having to crawl the docs tree. +- **An AI agent maintaining the library can locate the right code path from a single AGENTS.md read**, without having to crawl the docs tree. - **A security reviewer can audit the trust boundary by reading SPEC §15** and confirm every claim against `file:line` citations. ### 6.2 Quantitative diff --git a/RELEASING.md b/RELEASING.md index 088bf57..7252f77 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -58,10 +58,10 @@ Any item in that list should be either (a) in the changelog under ```bash pre-commit run --all-files pytest --cov=jvspatial --cov-fail-under=50 -mypy jvspatial/ +# The configured mypy pre-commit hook is part of pre-commit above. ``` -All three must be green. CI will re-run them on the PR — local runs +Both commands must be green. The standalone `mypy jvspatial/` invocation has pre-existing diagnostics outside the configured hook scope; track that debt separately. CI will re-run the gates on the PR — local runs just save a round trip. ### 3. Pick the new version number diff --git a/SPEC.md b/SPEC.md index bcd6306..8d605ee 100644 --- a/SPEC.md +++ b/SPEC.md @@ -7,7 +7,7 @@ > **Companion documents**: > - [PRD.md](PRD.md) — *why* the library exists (product context, users, success criteria) > - [ROADMAP.md](ROADMAP.md) — forward-looking direction and known gaps -> - [CLAUDE.md](CLAUDE.md) — operational guidance for AI agents maintaining this repo +> - [AGENTS.md](AGENTS.md) — operational guidance for AI agents maintaining this repo > - [docs/md/README.md](docs/md/README.md) — index of detailed how-to documentation > - [LLM-CODING-GUIDE.md](LLM-CODING-GUIDE.md) — usage cookbook (code patterns for callers) @@ -106,7 +106,7 @@ AttributeMixin + pydantic.BaseModel - Context lookup: `set_context()`, `get_context()` (default via `get_default_context()`) - Collection mapping via `get_collection_name()` → `{n: node, e: edge, o: object, w: walker}` -**Invariant**: `__setattr__` validates field names against the class hierarchy. Setting an undeclared attribute on an `Object` (post-init) is rejected. Prevents schema injection through attribute assignment. +**Invariant**: `__setattr__` validates field names against the class hierarchy. Setting an undeclared attribute on an `Object` (post-init) is rejected. Declared private attributes and helpers may be replaced on an instance; a new underscore name is still rejected (`jvspatial/core/entities/object.py:147`). Prevents schema injection through attribute assignment. ### 2.2 Node — graph node @@ -276,10 +276,14 @@ Callers branching on capabilities should test the flag, not the adapter class. | MongoDB | `jvspatial/db/mongodb.py` | Yes | `motor`; native bulk writes; native compound ops | | DynamoDB | `jvspatial/db/dynamodb.py` | No | `aioboto3`; throttle-retry; `BatchGetItem` chunks of 100 | +For a file-backed SQLite database reused on a different event loop, the adapter closes the old `aiosqlite` connection and its worker thread before opening a connection on the new loop. An in-memory SQLite database retains its existing connection so its contents are not lost (`jvspatial/db/sqlite.py`). + ### 4.4 Atomic IO (JSON) `jvspatial/db/_atomic.py` provides crash-safe writes: temp file → `fsync` → `rename` → `fsync(directory)`. Per-file mutex via `PathLockManager` (`_path_locks.py`) serializes concurrent writes to the same record. Bounded LRU prevents lock-table growth. +JsonDB collection names and record IDs must be single path components. Absolute paths, separators, and `..` are rejected before a record path is used; resolved paths must remain under `base_path` (`jvspatial/db/jsondb.py:138`, `jvspatial/db/jsondb.py:174`). + ### 4.5 Multi-database `jvspatial/db/manager.py` — one "prime" database for core ops (auth, sessions, API keys), plus additional databases for specialized use. Auth state is **always** on the prime database; this cannot be relocated. @@ -301,9 +305,9 @@ No built-in migration framework. Adapters do not enforce schemas. Adding optiona | `$eq`, `$ne` | Equality / inequality | | `$gt`, `$gte`, `$lt`, `$lte` | Comparison | | `$in`, `$nin` | Membership | -| `$exists` | Field presence | +| `$exists` | In-memory and PostgreSQL: a non-null value is present; missing and explicit JSON `null` count as absent. MongoDB retains native field-presence semantics, where explicit `null` counts as present. | | `$and`, `$or` | Logical combinators | -| `$regex` | Regex match (string fields). Never index-backed; build patterns from user input with `jvspatial.db.escape_regex` | +| `$regex` | Regex match (string fields). Never index-backed; build patterns from user input with `jvspatial.db.escape_regex`. In-memory matching caps pattern and candidate lengths and applies a 5 ms per-candidate timeout (`jvspatial/db/query.py`). Native database regex execution follows that database's resource limits. | | `$text` | Top-level `{"$text": {"$search": "words", "$fields": ["context.a", ...]}}`: every search word (case-insensitive, `\w+` tokens, no stemming) occurs in the concatenated fields; without `$fields` every string value is searched | ### 5.2 Pushdown vs in-memory @@ -352,6 +356,8 @@ When `protection_enabled=True` (default): Protection is *advisory* for safety, not for security — untrusted user input should not influence walker construction. +Cascade deletion stops and raises on the first edge or dependent-node failure. The parent is deleted only after all required deletions finish (`jvspatial/core/entities/node.py:1527`). + ### 6.4 Trail tracking `WalkerTrail` records `(node_id, edge_id, timestamp, node_type, queue_length, metadata)` for every visit (when `trail_enabled=True`). `max_trail_length=0` is unlimited; bounded by memory. In serverless deployments, trails do not persist across invocations. @@ -405,6 +411,8 @@ When an entity needs a context, it resolves in this order: `GraphContext` is request-scoped by convention. The API server installs a per-request context via middleware (`jvspatial/api/components/auth_middleware.py` and lifecycle), so endpoint handlers reach the correct database without manual injection. +`graph_transaction` uses an isolated request identity map while the transaction is open. On commit or rollback it evicts entities touched in the transaction from the parent context cache, so subsequent reads do not return pre-commit data or rolled-back objects (`jvspatial/core/context.py`). Index setup is cached per database instance; ensuring a model's indexes on one database does not suppress them on another. + ### 7.3 Performance monitoring `PerformanceMonitor` (within `GraphContext`) records: @@ -458,7 +466,7 @@ Endpoints decorated at import time are collected in a deferred registry (`api/en When `auth_enabled=True`, the server registers `/auth/register`, `/auth/login`, `/auth/logout`, plus token refresh and password reset endpoints. With `auth_enabled=False`, no auth endpoints are registered. -OpenAPI docs at `/docs` and `/redoc` unless `JVSPATIAL_DOCS_DISABLED` is truthy (see §10.5). +OpenAPI docs at `/docs` and `/redoc` in development unless `JVSPATIAL_DOCS_DISABLED` is truthy. In `JVSPATIAL_ENVIRONMENT=production`, docs are unpublished by default (see §10.5; `jvspatial/api/components/app_builder.py:62`). --- @@ -470,10 +478,13 @@ OpenAPI docs at `/docs` and `/redoc` unless `JVSPATIAL_DOCS_DISABLED` is truthy - **API keys**: SHA-256 hashed at rest, plaintext returned **only once** on creation. Verification uses `hmac.compare_digest` (constant-time). - **Refresh tokens**: rotate on use; previous tokens invalidated. - **Password reset**: `token_lookup` field provides O(1) lookup; constant-time comparison. +- **Live user check**: JWT decoding never authenticates without a loaded active user. User lookup errors reject the token; roles and permissions come from the loaded row, not JWT claims (`jvspatial/api/auth/service.py:1031`, `jvspatial/api/auth/service.py:1083`). ### 9.2 Authorization (RBAC) -`jvspatial/api/auth/rbac.py` — roles map to permission unions. Wildcard support (e.g. `users:*`). Admin-only routes enforced on `/status`, `/logs`, and `/graph` subtrees by default. +`jvspatial/api/auth/rbac.py` — roles map to permission unions. Wildcard support (e.g. `users:*`). Admin-only routes enforced on `/status`, `/logs`, and `/graph` subtrees by default. Endpoint-configuration lookup errors deny access; a successful lookup with no jvspatial metadata still requires authentication but has no undeclared role requirement, supporting mounted ASGI apps and raw FastAPI routes (`jvspatial/api/components/auth_middleware.py:153`). + +For a raw FastAPI route, the authentication middleware defers to FastAPI only when the route declares an enforcing `SecurityBase` dependency. Dependency names and optional security dependencies do not bypass framework authentication. Admin-only routes and routes with jvspatial endpoint metadata continue through framework authentication and RBAC (`jvspatial/api/components/endpoint_auth_resolver.py`). ### 9.3 Session management @@ -481,11 +492,15 @@ OpenAPI docs at `/docs` and `/redoc` unless `JVSPATIAL_DOCS_DISABLED` is truthy ### 9.4 Logout blacklist -JWT tokens are blacklisted on logout. Blacklist storage is per-worker in default config. Cross-worker invalidation requires a shared blacklist store (caller-provided). +JWT tokens are blacklisted on logout, and the bound refresh token is deactivated (`jvspatial/api/auth/service.py:978`). Blacklist lookup errors fail closed by default (`jvspatial/api/auth/service.py:493`). Blacklist storage is per-worker in default config. Cross-worker invalidation requires a shared blacklist store (caller-provided). ### 9.5 Webhook authentication -`jvspatial/api/integrations/webhooks/` — HMAC signature verification with constant-time comparison. Per-source secret rotation supported. Replay protection via timestamp window (configurable). +`jvspatial/api/integrations/webhooks/` — required HMAC signatures are checked on every HTTP method. GET signs the raw query string; body-bearing methods sign the raw body (`jvspatial/api/integrations/webhooks/middleware.py`). Comparison is constant-time. For requests with an idempotency header, the middleware atomically claims a deterministic key in the shared database before invoking the handler. A completed retry with identical method, path, and body replays the cached response. Pending claims and key reuse with different request content return 409. Claim or outcome storage failures fail closed; pending claims are retained for reconciliation, even after their TTL (`jvspatial/api/integrations/webhooks/{utils,middleware,models}.py`). + +### 9.6 OAuth signing keys + +When `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY` is set to a valid Fernet key, `OAuthSigningKey.save()` encrypts private PEM before persistence. The signing keystore returns a plaintext copy for signing while the stored row remains encrypted; a legacy plaintext row is rewrapped on first load. An encrypted row fails closed when the key is absent or incorrect. In `JVSPATIAL_ENVIRONMENT=production`, the OAuth keystore refuses to load or create keys without the encryption key. `build_jwks()` publishes only public material. Hosts with production OAuth must provision the encryption key and preserve it across workers and restarts. Key rotation and backup recovery require an operator procedure; this contract does not provide KMS/HSM custody (`jvspatial/api/auth/oauth/{keys,models}.py`). --- @@ -513,6 +528,8 @@ ServerConfig( ) ``` +Auth entry points have a per-IP 5-request/60-second cap by default, including when global rate limiting is disabled. Hosts with a trusted replacement limiter and test environments can set `RateLimitConfig(auth_entrypoint_rate_limit_enabled=False)` explicitly (`jvspatial/api/config_groups.py:344`, `jvspatial/api/server_configurator.py:173`). The global limiter setting alone does not disable this protection. + Flat keyword arguments (e.g. `Server(db_type=..., jwt_secret=...)`) are mapped to the appropriate group by a model validator (lines 103-128). The flat form is convenient; the hierarchical form is canonical. ### 10.2 Merge order @@ -537,7 +554,7 @@ Unknown `JVSPATIAL_*` keys are rejected at startup to catch typos and removed se ### 10.5 Docs gating -`JVSPATIAL_DOCS_DISABLED` (truthy values: `1`, `true`, `yes`, `on`) disables `/docs`, `/redoc`, `/openapi.json`, and `/docs/oauth2-redirect` at app build time. CSP headers are relaxed only on docs paths to allow the Swagger UI CDN; app routes retain strict CSP. +`JVSPATIAL_DOCS_DISABLED` (truthy values: `1`, `true`, `yes`, `on`) disables `/docs`, `/redoc`, `/openapi.json`, and `/docs/oauth2-redirect` at app build time. Its default is true when `JVSPATIAL_ENVIRONMENT=production`; an explicit false value re-enables docs. CSP headers are relaxed only on docs paths to allow the Swagger UI CDN; app routes retain strict CSP (`jvspatial/api/components/app_builder.py:62`). **Source of truth**: `jvspatial/api/components/app_builder.py`. @@ -599,6 +616,7 @@ When LWA is detected, `Server` applies best-effort defaults for `AWS_LWA_PASS_TH - `storage/security/path_sanitizer.py` — five-stage validation: regex blocklist (11 patterns), normalization with re-check, hidden-file allowlist, symlink resolution, base-directory confinement. - `storage/security/validator.py` — content-based MIME via `python-magic`. ~25 allowed types, 14 blocked types, 19 blocked extensions. Internal markers bypassed via metadata validation only (never user-supplied). +- Local version IDs must be one filename component; version files and pointers must resolve inside the storage root (`jvspatial/storage/interfaces/local.py:192`). ### 12.2 Upload contract @@ -664,6 +682,7 @@ User input crosses the trust boundary at: 2. **`Entity.update()`** — validates all field names against the class hierarchy; rejects undeclared attributes (`object.py` setter). 3. **File uploads** — content-based MIME validation, not extension-based. 4. **Path inputs** — `path_sanitizer.py` before any filesystem access. +5. **Index definitions** — SQLite and Postgres accept safe field-path segments and translated partial-filter expressions; raw `where=` SQL is rejected (`jvspatial/db/sqlite.py:344`, `jvspatial/db/postgres.py:1421`). ### 15.2 Constant-time comparisons @@ -792,7 +811,7 @@ Control-flow exceptions for walkers live in `jvspatial.core.entities`: - Database errors propagate to the caller; entity methods may wrap with context. - Validation errors raise `ValidationError` with `field_errors`. The API layer maps to HTTP 400. - Authentication errors map to HTTP 401; authorization errors to HTTP 403. -- Walker errors are logged and surface in `walker.response["errors"]`; traversal halts. +- Non-protection walker errors are reported and raised as `WalkerExecutionError`; protection violations retain their specialized errors (`jvspatial/core/entities/walker.py:902`). --- diff --git a/docs/md/README.md b/docs/md/README.md index 595d272..4b2735d 100644 --- a/docs/md/README.md +++ b/docs/md/README.md @@ -6,7 +6,7 @@ This directory holds detailed how-to and reference documentation. For higher-lev - [PRD.md](../../PRD.md) — *why* the library exists, target users, non-goals - [SPEC.md](../../SPEC.md) — *what* the library guarantees (technical contract) - [ROADMAP.md](../../ROADMAP.md) — forward direction and known gaps -- [CLAUDE.md](../../CLAUDE.md) — agent maintenance guide +- [AGENTS.md](../../AGENTS.md) — agent maintenance guide - [CHANGELOG.md](../../CHANGELOG.md) — release history The documents below answer *how* to use each subsystem. Every link resolves; entries marked **NEW** were added since the previous index revision. @@ -27,7 +27,7 @@ The documents below answer *how* to use each subsystem. Every link resolves; ent | Document | What's in it | |---|---| -| [Entity Reference](entity-reference.md) | `Object`, `Node`, `Edge`, `Walker`, `Root` — fields, lifecycle, persistence shape. | +| [Entity Reference](entity-reference.md) | `Object`, `Node`, `Edge`, `Walker`, `Root` — fields, protected assignment, lifecycle, persistence shape. | | [Attribute Annotations](attribute-annotations.md) | `@attribute(protected, transient, private, indexed, …)` semantics. | | [Graph Context](graph-context.md) | Database + cache + monitor binding; multi-database setup. | | [Context Management Guide](context-management-guide.md) | When and how to scope `GraphContext` / `ServerContext`. | @@ -48,7 +48,7 @@ The documents below answer *how* to use each subsystem. Every link resolves; ent | [API Architecture](api-architecture.md) | Mixin composition, request lifecycle, middleware stack. | | [Decorator Reference](decorator-reference.md) | Every decorator the library ships: `@endpoint`, `@attribute`, `@on_visit`, etc. | | [Pagination](pagination.md) | `ObjectPager` usage. | -| [Rate Limiting](rate-limiting.md) | **NEW** Token-bucket rate limit configuration. | +| [Rate Limiting](rate-limiting.md) | Global and endpoint limits, including the independent auth-entrypoint cap. | | [Error Handling](error-handling.md) | Exception taxonomy and propagation. | ## Authentication and Security diff --git a/docs/md/auth-quickstart.md b/docs/md/auth-quickstart.md index cda3231..244fe39 100644 --- a/docs/md/auth-quickstart.md +++ b/docs/md/auth-quickstart.md @@ -8,7 +8,7 @@ Get your jvspatial API secured in 5 minutes with comprehensive authentication in ## Prerequisites - jvspatial installed: `pip install jvspatial` -- Python 3.8+ environment +- Python 3.9+ environment ## Step 1: Basic Setup (2 minutes) @@ -45,7 +45,7 @@ if __name__ == "__main__": ``` **That's it!** Your server now has full authentication with: -- User registration (`POST /api/auth/register`) — first user becomes admin (bootstrap) +- User registration (`POST /api/auth/register`) — creates a regular user when registration is open - Current user (`GET /api/auth/me`) — returns authenticated user - User login (`POST /api/auth/login`) - JWT token validation and RBAC (roles and permissions) @@ -87,30 +87,30 @@ async def advanced_ops(): ## Step 3: Create Your First User (1 minute) -### First-user bootstrap (automatic) +### Admin bootstrap -When no users exist, `POST /api/auth/register` creates the first user and assigns the `admin` role. This is the recommended way to bootstrap your application. +Public registration never assigns the `admin` role. Set `ADMIN_EMAIL` and `ADMIN_PASSWORD` (at least 12 characters) before first startup to bootstrap an admin. `bootstrap_admin_email` and `bootstrap_admin_password` in the server configuration use these values. Public registration is available only when `registration_open` is enabled. -### Option A: Via API (Recommended) +### Option A: Register a regular user via API ```bash # Start your server python auth_server.py -# Register first user (becomes admin automatically) +# Register a regular user curl -X POST "http://localhost:8000/api/auth/register" \ -H "Content-Type: application/json" \ - -d '{"email": "admin@example.com", "password": "admin123"}' + -d '{"email": "user@example.com", "password": "example-password-123"}' # Login to get token curl -X POST "http://localhost:8000/api/auth/login" \ -H "Content-Type: application/json" \ - -d '{"email": "admin@example.com", "password": "admin123"}' + -d '{"email": "user@example.com", "password": "example-password-123"}' ``` ### Option B: Admin creates users (after bootstrap) -Once users exist, public registration is disabled. Admins create users via: +To disable public registration, set `registration_open=False`. Admins can create users via: ```bash # Admin creates a new user with roles @@ -119,7 +119,7 @@ curl -X POST "http://localhost:8000/api/auth/admin/users" \ -H "Content-Type: application/json" \ -d '{ "email": "user@example.com", - "password": "user123", + "password": "example-password-123", "roles": ["user"], "permissions": [] }' @@ -168,7 +168,7 @@ if __name__ == "__main__": server.run() ``` -Run it: `python quickstart.py`. Register the first user via `POST /api/auth/register` — they become admin automatically. Visit: http://localhost:8000/docs +Run it: `python quickstart.py`. Register a regular user via `POST /api/auth/register`. Configure bootstrap admin credentials before startup when admin access is needed. Visit: http://localhost:8000/docs ## Advanced Features (Optional) @@ -349,4 +349,4 @@ export JVSPATIAL_DB_PATH="./data" **Total setup time: ~5 minutes** -Your jvspatial API is now secured with enterprise-grade authentication! \ No newline at end of file +Your jvspatial API is now secured with enterprise-grade authentication! diff --git a/docs/md/authentication.md b/docs/md/authentication.md index d63c409..a549f8d 100644 --- a/docs/md/authentication.md +++ b/docs/md/authentication.md @@ -731,9 +731,9 @@ The authentication middleware uses the **endpoint registry** as the single sourc - `auth=True`: Endpoint requires authentication - `auth=False`: Endpoint is public (no authentication required) - Default: `auth=False` (endpoints are public by default) -3. **Unregistered Endpoints**: Endpoints not in the registry **require authentication by default** (deny by default security model) +3. **Unregistered Endpoints**: Endpoints not in the registry, including mounted ASGI apps and raw FastAPI routes, **require authentication by default**. A successful metadata lookup with no jvspatial endpoint configuration does not invent a role requirement; a lookup error denies access. Built-in `/status`, `/logs`, and `/graph` subtrees still require admin roles. -**Important**: Only endpoints explicitly registered with `auth=False` are public. This ensures that: +**Important**: Outside the documented exempt paths, only endpoints explicitly registered with `auth=False` are public. This ensures that: - Dynamically registered endpoints (e.g., from extended applications) respect their `auth` settings - Public endpoints with `auth=False` are accessible without authentication - The authentication behavior is consistent across all registered endpoints @@ -744,9 +744,9 @@ The authentication middleware uses the **endpoint registry** as the single sourc The authentication middleware follows a **"deny by default"** security model: 1. **Exempt Paths**: Paths in `exempt_paths` bypass authentication. Built-in auth paths (`/auth/register`, `/auth/login`, `/auth/refresh`, `/auth/logout`, `/auth/signup`, `/auth/forgot-password`, `/auth/reset-password`) are always exempt. **`/_internal/deferred`** (jvspatial deferred task / LWA pass-through) is **always** merged in as well—you cannot remove it via `exempt_paths`, because Lambda async invoke cannot send your app JWT. PathMatcher expands these using `APIRoutes.PREFIX` (default `/api`). -2. **Files HTTP routes** (`FileStorageService`): Registered under **`{JVSPATIAL_API_PREFIX}/files`** (default `/api/files`). **`POST .../files/upload`**, **`DELETE .../files/{path}`**, and proxy admin routes under **`.../files/proxy`** require JWT or API key when auth is enabled. **`GET .../files/{path}`** is **public by default**; set **`JVSPATIAL_FILES_PUBLIC_READ=false`** to require authentication for direct reads. **`GET {JVSPATIAL_PROXY_PREFIX}/{code}`** (default `/p/{code}`) is explicitly public—the proxy code is the credential. For anonymous direct file URLs when public read is off, use proxy links or add a **narrow** `exempt_paths` pattern only after security review (patterns are path-only, not per HTTP method). +2. **Files HTTP routes** (`FileStorageService`): Registered under **`{JVSPATIAL_API_PREFIX}/files`** (default `/api/files`). **`POST .../files/upload`**, **`DELETE .../files/{path}`**, and proxy admin routes under **`.../files/proxy`** require JWT or API key when auth is enabled. **`GET .../files/{path}`** also requires authentication by default when auth is enabled; set **`JVSPATIAL_FILES_PUBLIC_READ=true`** to allow direct public reads. **`GET {JVSPATIAL_PROXY_PREFIX}/{code}`** (default `/p/{code}`) is explicitly public—the proxy code is the credential. For anonymous direct file URLs when public read is off, use proxy links or add a **narrow** `exempt_paths` pattern only after security review (patterns are path-only, not per HTTP method). 3. **Registered Endpoints**: Endpoints in the registry with `auth=False` are public -4. **Unknown Endpoints**: Endpoints not in the registry **require authentication** +4. **Unknown Endpoints**: Endpoints not in the registry **require authentication**. Mounted apps and raw FastAPI routes remain reachable to authenticated callers unless another policy restricts them; built-in administrative subtrees retain their admin-role check. 5. **Error Handling**: Any error during authentication checking **denies access** This approach ensures that: @@ -957,4 +957,4 @@ server = Server( --- -**[← Back to README](../../README.md)** | **[Authentication Examples →](../examples/api/authenticated_endpoints_example.py)** \ No newline at end of file +**[← Back to README](../../README.md)** | **[Authentication Examples →](../examples/api/authenticated_endpoints_example.py)** diff --git a/docs/md/entity-reference.md b/docs/md/entity-reference.md index 7e52d9e..a73eb80 100644 --- a/docs/md/entity-reference.md +++ b/docs/md/entity-reference.md @@ -46,6 +46,8 @@ create. See `Database.insert_if_absent` / `InsertIfAbsentResult`. - `await Object.find_one({"context.email": "alice@example.com"})` → find single object matching query (returns None if not found) - `await Object.find_one(email="alice@example.com")` → find single object using keyword arguments +**Attribute assignment:** `Object` rejects names absent from its class hierarchy, including new underscore-prefixed names. Persisted fields should be declared with `@attribute(...)`; runtime-only state should use Pydantic `PrivateAttr`. A private method or descriptor already declared on the class may be replaced on an instance (for example, by a test double), but assigning an undeclared `_helper` still raises `AttributeError`. See [SPEC §2.1](../../SPEC.md#21-object--base-persistable-entity). + #### `Node(Object)` Represents graph nodes with connection capabilities. diff --git a/docs/md/environment-configuration.md b/docs/md/environment-configuration.md index e3dd85b..4e9131c 100644 --- a/docs/md/environment-configuration.md +++ b/docs/md/environment-configuration.md @@ -135,7 +135,7 @@ export JVSPATIAL_TEXT_NORMALIZATION_ENABLED=false Removed JWT env names (do not use): `JVSPATIAL_JWT_EXPIRATION_HOURS`, `JVSPATIAL_JWT_REFRESH_EXPIRATION_DAYS`. | `ADMIN_EMAIL` | string | — | Admin email for bootstrap. When set with `ADMIN_PASSWORD`, creates an admin user on first run. Pass to `auth.bootstrap_admin_email`. | -| `ADMIN_PASSWORD` | string | — | Admin password for bootstrap (min 6 chars). Pass to `auth.bootstrap_admin_password`. | +| `ADMIN_PASSWORD` | string | — | Admin password for bootstrap (min 12 chars). Pass to `auth.bootstrap_admin_password`. | | `ADMIN_NAME` | string | — | Admin display name for bootstrap. Defaults to email. Pass to `auth.bootstrap_admin_name`. | Example usage in Server config: @@ -560,4 +560,4 @@ if __name__ == "__main__": ctx = configure_production() ``` -This configuration approach ensures reliable, secure, and maintainable deployments across different environments. \ No newline at end of file +This configuration approach ensures reliable, secure, and maintainable deployments across different environments. diff --git a/docs/md/environment-keys-reference.md b/docs/md/environment-keys-reference.md index 97d19bf..bbb66c2 100644 --- a/docs/md/environment-keys-reference.md +++ b/docs/md/environment-keys-reference.md @@ -31,7 +31,7 @@ For full examples and default values, see: - `JVSPATIAL_API_HEALTH` - Health route path. - `JVSPATIAL_API_ROOT` - Root route path. - `JVSPATIAL_GRAPH_ENDPOINT_ENABLED` - Enables graph REST endpoint. -- `JVSPATIAL_DOCS_DISABLED` - When truthy (`1`/`true`/`yes`/`on`), unpublishes the documentation surface entirely: `/docs`, `/redoc`, `/openapi.json`, and `/docs/oauth2-redirect` are not registered (return 404). Recommended for production. +- `JVSPATIAL_DOCS_DISABLED` - When truthy (`1`/`true`/`yes`/`on`), unpublishes `/docs`, `/redoc`, `/openapi.json`, and `/docs/oauth2-redirect`. Defaults to true when `JVSPATIAL_ENVIRONMENT=production`; explicit false overrides it. ### CORS - `JVSPATIAL_CORS_ENABLED` - Enables CORS middleware. @@ -56,10 +56,12 @@ For full examples and default values, see: ### Auth and rate limit - `JVSPATIAL_AUTH_ENABLED` - Enables auth. - `JVSPATIAL_JWT_SECRET_KEY` - JWT signing secret. +- `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY` - Fernet key for OAuth RSA private keys at rest. Supply from a secret manager in production; retain the same key across workers and restarts. - `JVSPATIAL_JWT_ALGORITHM` - JWT algorithm. - `JVSPATIAL_JWT_EXPIRE_MINUTES` - Access token expiry (minutes). - `JVSPATIAL_JWT_REFRESH_EXPIRE_DAYS` - Refresh token expiry (days). -- `JVSPATIAL_AUTH_STRICT_HASHING` - Disables weak hashing fallback. +- `JVSPATIAL_AUTH_STRICT_HASHING` - Disables weak hashing fallback for passwords and refresh tokens; true by default. +- `JVSPATIAL_AUTH_BLACKLIST_FAIL_CLOSED` - Rejects tokens when blacklist lookup fails; true by default. - `JVSPATIAL_BCRYPT_ROUNDS` - Bcrypt rounds. - `JVSPATIAL_BCRYPT_ROUNDS_SERVERLESS` - Bcrypt rounds in serverless. - `JVSPATIAL_ARGON2_TIME_COST` - Argon2 tuning. @@ -85,7 +87,7 @@ For full examples and default values, see: - `JVSPATIAL_FILE_STORAGE_BASE_URL` - Public base URL for files. - `JVSPATIAL_FILE_STORAGE_MAX_SIZE` - Max upload size (bytes). - `JVSPATIAL_FILE_STORAGE_SERVERLESS_SHARED` - Marks local path as durable shared storage. -- `JVSPATIAL_FILES_PUBLIC_READ` - Public `GET` access for file routes. +- `JVSPATIAL_FILES_PUBLIC_READ` - Public `GET` access for file routes; false by default when auth is enabled. - `JVSPATIAL_FILE_INTERFACE` - Default storage interface selection. - `JVSPATIAL_S3_BUCKET_NAME` - S3 bucket. - `JVSPATIAL_S3_REGION` - S3 region. @@ -141,6 +143,7 @@ For full examples and default values, see: - `JVSPATIAL_AWS_SQS_QUEUE_URL` - SQS queue URL when SQS transport is used. - `JVSPATIAL_DEFERRED_INVOKE_DISABLED` - Disable deferred invoke route mount. - `JVSPATIAL_DEFERRED_INVOKE_SECRET` - Authorization secret for deferred invoke route. +- `JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK` - Explicitly allows Lambda Web Adapter loopback self-invoke without a secret; false by default. - `JVSPATIAL_WORK_CLAIM_STALE_SECONDS` - Claim lease TTL in seconds. ### EventBridge scheduler diff --git a/docs/md/file-storage-usage.md b/docs/md/file-storage-usage.md index ae6b253..23f484c 100644 --- a/docs/md/file-storage-usage.md +++ b/docs/md/file-storage-usage.md @@ -31,7 +31,7 @@ The jvspatial file storage system provides secure, scalable file management with - **HTTP paths** are **`{JVSPATIAL_API_PREFIX}/files/...`** (default **`/api/files/...`**): upload is **`POST /api/files/upload`**, direct file access is **`GET /api/files/{file_path}`**, proxy admin is under **`/api/files/proxy`**. There is no separate storage prefix env var. - **OpenAPI** groups these operations under the **`Files`** tag. -- **Authentication**: When `auth` middleware is enabled, **`POST /api/files/upload`**, **`DELETE /api/files/{path}`**, and proxy admin routes require a JWT or API key. **`GET /api/files/{path}`** is **public by default**; set **`JVSPATIAL_FILES_PUBLIC_READ=false`** to require auth for direct reads. **`GET {JVSPATIAL_PROXY_PREFIX}/{code}`** (default `/p/{code}`) stays public: the opaque code and expiry act as the credential. For anonymous third-party fetch when public read is off, use **proxy URLs** or add a **narrow** `auth.exempt_paths` entry (path-only patterns cannot distinguish GET vs POST; review security before exempting broad prefixes). +- **Authentication**: When `auth` middleware is enabled, **`POST /api/files/upload`**, **`DELETE /api/files/{path}`**, proxy admin routes, and **`GET /api/files/{path}`** require a JWT or API key by default. Set **`JVSPATIAL_FILES_PUBLIC_READ=true`** only when direct public reads are intentional. **`GET {JVSPATIAL_PROXY_PREFIX}/{code}`** (default `/p/{code}`) stays public: the opaque code and expiry act as the credential. For anonymous third-party fetch, use **proxy URLs** or add a **narrow** `auth.exempt_paths` entry (path-only patterns cannot distinguish GET vs POST; review security before exempting broad prefixes). ### Key Features @@ -1682,4 +1682,4 @@ For technical architecture details, see [File Storage Architecture](file-storage --- **Last Updated:** 2025-01-05 -**Version:** 1.0.0 \ No newline at end of file +**Version:** 1.0.0 diff --git a/docs/md/mongodb-query-interface.md b/docs/md/mongodb-query-interface.md index 4eb7618..80a8d61 100644 --- a/docs/md/mongodb-query-interface.md +++ b/docs/md/mongodb-query-interface.md @@ -2,6 +2,8 @@ The jvspatial library provides a unified **MongoDB-style query interface** that works consistently across all database backends. This allows you to use familiar MongoDB query syntax regardless of whether you're using JSON files, MongoDB, or custom database implementations. +`$exists` is a known semantic difference: MongoDB counts an explicit `null` field as present, while the in-memory matcher and PostgreSQL count it as absent. Avoid relying on `$exists` for nullable fields when switching backends. + ## Overview The query interface provides: diff --git a/docs/md/production-deployment.md b/docs/md/production-deployment.md index 8e4613d..5b432d7 100644 --- a/docs/md/production-deployment.md +++ b/docs/md/production-deployment.md @@ -23,7 +23,7 @@ JVSPATIAL_JWT_SECRET_KEY=your-cryptographically-secure-secret-minimum-32-chars ### 2. Rate Limiting -Rate limiting is **disabled by default**. For production, enable it to protect against brute-force attacks and DoS. **Especially important** when using forgot-password: enable rate limiting to prevent abuse of the public `/auth/forgot-password` endpoint. +Global rate limiting is **disabled by default**, but auth-enabled servers still cap register, login, forgot-password, and reset-password at 5 requests per 60 seconds per IP. For production, enable the global limiter for the rest of the API and use a shared rate-limit backend across workers or hosts. Keep the auth cap enabled unless a trusted host limiter provides equivalent protection. See the [rate-limiting guide](rate-limiting.md#authentication-entry-points). ```python server = Server( @@ -117,6 +117,7 @@ JVSPATIAL_WEBHOOK_HMAC_SECRET=your-webhook-secret-minimum-32-chars | Variable | Production Recommendation | |----------|---------------------------| | `JVSPATIAL_JWT_SECRET_KEY` | **Required** - cryptographically secure, 32+ chars | +| `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY` | **Required when OAuth is enabled** - Fernet key from a secret manager; see [key custody](security-operational-notes.md#oauth-signing-key-custody) | | `JVSPATIAL_RATE_LIMIT_ENABLED` | `true` | | `JVSPATIAL_DEBUG` | `false` | | `JVSPATIAL_LOG_LEVEL` | `info` or `warning` | diff --git a/docs/md/rate-limiting.md b/docs/md/rate-limiting.md index 7c765fe..fd54f64 100644 --- a/docs/md/rate-limiting.md +++ b/docs/md/rate-limiting.md @@ -16,6 +16,24 @@ Rate limiting controls how many requests a client can make within a specific tim ## Configuration +### Authentication entry points + +When authentication is enabled, register, login, forgot-password, and reset-password have a 5-request/60-second per-IP cap by default. This cap is independent of `rate_limit_enabled`: turning off the global limiter does not turn off auth protection. The routes use the configured API prefix (normally `/api`). + +An application with its own trusted auth limiter, or a test server that needs to make repeated auth requests, can opt out explicitly: + +```python +from jvspatial.api import Server +from jvspatial.api.config_groups import RateLimitConfig + +server = Server( + auth={"auth_enabled": True}, + rate_limit=RateLimitConfig(auth_entrypoint_rate_limit_enabled=False), +) +``` + +Keep the default in production unless the replacement limiter provides equivalent protection. In-memory counters are local to each worker; use a shared backend when the cap must apply across workers or hosts. See [security operational notes](security-operational-notes.md#in-memory-rate-limiting-and-auth-rate-helpers). + ### Enable Rate Limiting ```python diff --git a/docs/md/security-operational-notes.md b/docs/md/security-operational-notes.md index bcea311..127a385 100644 --- a/docs/md/security-operational-notes.md +++ b/docs/md/security-operational-notes.md @@ -2,6 +2,10 @@ This page complements [environment-configuration.md](environment-configuration.md) with deployment-facing security behavior. +## OAuth signing key custody + +Set `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY` to a Fernet key from your secret manager before enabling OAuth in production. Generate one with `python -c 'from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())'`. In `JVSPATIAL_ENVIRONMENT=production`, the keystore refuses to load or create a key without it. The keystore encrypts new private keys before persistence and rewraps a legacy plaintext key when it is first loaded. Back up the encryption key separately from the database, keep it identical across workers, and restrict both secret and database access. Missing or incorrect keys cause encrypted-key signing to fail closed. A database backup taken before the first rewrap still contains plaintext and must be protected or replaced. This is application-level encryption, not a managed KMS/HSM integration; key rotation requires an explicit re-encryption procedure before retiring the old key. + ## Redis cache - Use a **dedicated Redis instance** (or logical database + ACLs) per application. Do not share the same keyspace with untrusted writers. @@ -13,10 +17,14 @@ This page complements [environment-configuration.md](environment-configuration.m Webhook authentication can read API keys from query parameters or path segments (see webhook configuration). **Prefer header-based API keys in production.** Query and path parameters are more likely to appear in access logs, reverse proxies, browser history, and `Referer` headers. -## JWT blacklist (fail-open) +Webhook idempotency keys are claimed atomically in the shared database before the handler runs. An in-flight retry or reused key with different request content returns 409. If a worker dies after a claim, the outcome may be uncertain; inspect the downstream effect before clearing the pending record. A database write failure returns 503 rather than relying on process-local memory. Claims only guard requests that supply an idempotency header. Keep webhook side effects idempotent at their own boundary when practical. -If the database or cache path used for token blacklist checks raises an error, validation **fails open**: the token is treated as **not** blacklisted so the API stays available. Failures are logged at **ERROR** with stack traces—monitor these logs in production because revocation may be ineffective until the underlying issue is fixed. +## JWT blacklist (fail-closed) + +If the database or cache path used for token blacklist checks raises an error, validation **fails closed** by default: the token is treated as blacklisted. Failures are logged at **ERROR** with stack traces. `JVSPATIAL_AUTH_BLACKLIST_FAIL_CLOSED=false` restores the previous availability behavior and should be used only with a documented acceptance of revocation risk. Use a shared session store across workers for prompt cross-worker revocation. ## In-memory rate limiting and auth rate helpers In-process counters (for example `MemoryRateLimitBackend` and in-memory auth attempt tracking) **do not synchronize across workers or hosts**. For multiple uvicorn workers, Kubernetes replicas, or autoscaling groups, use a **shared backend** (for example Redis-backed rate limiting) so limits apply globally. + +Auth entry points keep their 5/60s per-IP cap when global rate limiting is off. Set `rate_limit.auth_entrypoint_rate_limit_enabled=False` only in tests or when the host application supplies an equivalent trusted limiter. This is an explicit server configuration choice; disabling the global limiter does not turn it off (`jvspatial/api/server_configurator.py:95`, `jvspatial/api/server_configurator.py:173`). diff --git a/docs/md/security-review.md b/docs/md/security-review.md index 5116334..4017fc5 100644 --- a/docs/md/security-review.md +++ b/docs/md/security-review.md @@ -1,4 +1,4 @@ -# jvspatial — Security Code Review (Final) +# jvspatial — Security Code Review **Date:** 2026-05-02 **Reviewer:** Claude Code (primary) + Explore agent (parallel scan) @@ -6,11 +6,65 @@ **Scope:** Full codebase — `jvspatial/` package, authentication, storage, API middleware, database backends, webhooks, scheduler, serverless **Prior reviews:** 2026-05-01 (13 findings, all remediated) → 2026-05-02 reassessment (7 findings, all remediated) +## 2026-09-27 combined gap and security review + +The consumer release pass found two cross-backend consistency defects: PostgreSQL `$exists: false` excluded explicit JSON `null`, and a graph transaction left pre-commit entities in the parent cache. The query translator now matches in-memory null semantics; the transaction isolates its identity map and evicts touched parent entries after its outcome. Index setup is cached per database instance so a second store cannot silently miss a unique index. PostgreSQL integration and transaction cache regressions cover these paths in `tests/db/test_postgres_integration.py` and `tests/core/test_graph_transaction.py`. + +The full framework suite also exposed an `aiosqlite` worker thread left alive after file-backed SQLite rebinding across event loops. Rebinding now closes the prior connection, and `tests/db/test_sqlite_cross_loop_audit.py` verifies the old thread stops. + +The pre-merge consumer assessment also identified plaintext OAuth RSA private keys in the persisted `OAuthSigningKey` row. The keystore now encrypts new keys when `JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY` is configured, rewraps existing plaintext rows on first load, and fails closed for a missing or incorrect key on encrypted rows. Tests cover encryption, migration, and wrong-key failure in `tests/api/auth/oauth/test_oauth_keys.py`. Production hosts must provide and safeguard the key; managed KMS/HSM custody and routine rotation remain operational work. + +The September review found 37 additional items. The May conclusion below describes the May review only. The numbered entries here correspond to the combined backlog; the code and regression tests in this PR are the remediation evidence. + +The pre-merge 0.1.0 review also found a short catastrophic-backtracking `$regex` pattern, a name-based FastAPI authentication bypass, and an idempotency path that had no durable claim and could silently fall back to memory. The fixes and regressions are in `jvspatial/db/query.py`, `jvspatial/api/components/endpoint_auth_resolver.py`, `jvspatial/api/integrations/webhooks/{utils,middleware,models}.py`, and `tests/security/test_combined_review_regressions.py`. Operational handling of uncertain webhook outcomes is described in [security-operational-notes.md](security-operational-notes.md). + +| # | Remediation | Evidence | +|---|---|---| +| 1 | User lookup errors reject JWTs | `jvspatial/api/auth/service.py:1031`; `tests/security/test_combined_review_regressions.py` | +| 2 | Current user roles and permissions replace JWT claims; role updates revoke tokens | `jvspatial/api/auth/service.py:1083`, `jvspatial/api/auth/service.py:1530` | +| 3 | Revocation writes the active session store and surfaces write errors | `jvspatial/api/auth/service.py:1220`, `jvspatial/api/auth/service.py:1270`; `jvspatial/api/auth/_session_store.py:82` | +| 4 | Logout deactivates the refresh token bound to the access JTI | `jvspatial/api/auth/service.py:974` | +| 5 | Password reset marks the token used before the password write and distinguishes later revocation failure | `jvspatial/api/auth/service.py:1425` | +| 6 | Config lookup errors deny requests; admin-only routes retain required roles even without metadata | `jvspatial/api/components/auth_middleware.py:153`; `jvspatial/api/components/endpoint_auth_resolver.py:356` | +| 7 | Signed GET webhooks require HMAC; webhook decorator defaults to POST | `jvspatial/api/integrations/webhooks/middleware.py:348`; `jvspatial/api/decorators/route.py:169` | +| 8 | Local version IDs and paths are confined to the root | `jvspatial/storage/interfaces/local.py:192` | +| 9 | JsonDB collection and record paths are confined to `base_path` | `jvspatial/db/jsondb.py:138` | +| 10 | API-key allowlists match exact paths or complete wildcard segments | `jvspatial/api/components/auth_middleware.py:468` | +| 11 | Public registration always assigns the default role | `jvspatial/api/auth/service.py:775` | +| 12 | Handled 500 responses use generic details | `jvspatial/api/components/error_handler.py:616`; `graph_visualization.py`; `storage/service.py` | +| 13 | Proxy manager calls are awaited and URLs use returned codes | `jvspatial/api/integrations/storage/service.py:110` | +| 14 | Node and edge filters use `_entity_name()` | `jvspatial/core/entities/node_query.py:50` | +| 15 | Cascade deletion raises on edge or child failure | `jvspatial/core/entities/node.py:1527` | +| 16 | Traversal errors raise after reporting | `jvspatial/core/entities/walker.py:900` | +| 17 | Blacklist lookup fails closed by default | `jvspatial/api/auth/service.py:144` | +| 18 | File reads are private by default with auth enabled | `jvspatial/api/integrations/storage/service.py:311` | +| 19 | Auth entry points have per-IP rate limits; account-state errors are generic | `jvspatial/api/server_configurator.py:95`; `auth_configurator.py` | +| 20 | Cached webhook API keys are checked for active state and expiry | `jvspatial/api/integrations/webhooks/webhook_auth.py:186` | +| 21 | Deferred invoke requires a secret on loopback unless explicitly exempted for LWA | `jvspatial/api/deferred_invoke_route.py:58` | +| 22 | Refresh-token hashing follows the strict hashing default | `jvspatial/api/auth/service.py:339` | +| 23 | SQL index paths are validated; raw `where=` is rejected | `jvspatial/db/sqlite.py:343`; `postgres.py:1421` | +| 24 | Production docs are unpublished by default | `jvspatial/api/components/app_builder.py:61` | +| 25 | New passwords require at least 12 characters | `jvspatial/api/auth/models.py:20`; `service.py` | +| 26 | Webhook handlers and idempotency storage complete within the request | `jvspatial/api/integrations/webhooks/middleware.py:215` | +| 27 | DynamoDB bulk writes raise on unprocessed items | `jvspatial/db/dynamodb.py:838` | +| 28 | Atomic increment validates fields before writing and returns false for missing fields | `jvspatial/core/context.py:1114` | +| 29 | Root lock is created per event loop | `jvspatial/core/entities/root.py:19` | +| 30 | Exported `retry` awaits and retries coroutines | `jvspatial/utils/decorators.py:150` | +| 31 | Admin bootstrap aborts on unreadable user rows | `jvspatial/api/auth/service.py:847` | +| 32 | Webhook idempotency database errors return 503 | `jvspatial/api/integrations/webhooks/utils.py:392` | +| 33 | Trail append tasks are retained and failures logged; async traversal awaits append | `jvspatial/core/entities/walker_components/walker_trail.py:70`; `walker.py` | +| 34 | Scheduler coroutines run on the server event loop | `jvspatial/api/integrations/scheduler/scheduler.py:608` | +| 35 | Regex patterns and values are bounded before evaluation | `jvspatial/db/query.py:512` | +| 36 | Underscore attributes must be declared on the class hierarchy; declared private helpers can be replaced | `jvspatial/core/entities/object.py:157` | +| 37 | Incorrect DeferredSaveMixin MRO raises at class creation | `jvspatial/core/mixins/deferred_save.py:157` | + +Security-sensitive changes to auth and secrets are entries 1–7, 10–11, 17, 19–22, 25, and 31–32. Each has a code location above. Deployment decisions for shared revocation storage, session and rate-limit stores, schema migrations, and database transactions remain in `ROADMAP.md` §2. + --- ## Executive Summary -This is the final security assessment of jvspatial. All 20 findings across two review cycles (13 from 2026-05-01, 7 from 2026-05-02 reassessment) have been implemented, verified, and confirmed passing the full test suite. **Zero remaining security findings.** +At the close of the May 2026 review, all 20 findings across two cycles (13 from 2026-05-01, 7 from 2026-05-02 reassessment) had been implemented and verified. The September 2026 combined review above identified additional findings and records their remediation. The codebase is in production-ready security condition with mature, defense-in-depth design across authentication, storage, webhooks, walker protection, and configuration validation. @@ -183,4 +237,4 @@ All findings were cross-referenced between passes and verified against current c ## Conclusion -**Zero remaining security findings.** All 20 issues across two review cycles have been remediated, verified, and confirmed passing the full test suite. The codebase demonstrates mature, defense-in-depth security design across authentication, storage, webhooks, walker protection, rate limiting, and configuration validation. jvspatial is in production-ready security condition. +All 20 issues in the May 2026 review were remediated at that time. See the September 2026 combined review above for later findings and fixes. Deployment readiness still depends on the operator's shared-store and infrastructure choices documented in `ROADMAP.md` §2. diff --git a/docs/md/server-api.md b/docs/md/server-api.md index 6d5b9ce..0d17c06 100644 --- a/docs/md/server-api.md +++ b/docs/md/server-api.md @@ -146,6 +146,10 @@ The `Server` class is the main entry point for creating jvspatial-powered APIs. ## Configuration +### Auth entrypoint rate limit + +For auth-enabled servers, `RateLimitConfig.auth_entrypoint_rate_limit_enabled` defaults to `True`. Register, login, forgot-password, and reset-password each retain a 5-request/60-second per-IP cap even if `rate_limit_enabled=False`. Set the flag to `False` explicitly only in tests or when the host provides an equivalent trusted limiter. See the [rate-limiting guide](rate-limiting.md#authentication-entry-points) for the configuration example and shared-backend considerations. + ### Basic Configuration ```python diff --git a/docs/md/serverless-mode.md b/docs/md/serverless-mode.md index 6222991..b3fd37f 100644 --- a/docs/md/serverless-mode.md +++ b/docs/md/serverless-mode.md @@ -118,7 +118,7 @@ Direct Lambda invocations (async invoke and EventBridge targets) deliver a JSON - **Canonical path**: `{JVSPATIAL_API_PREFIX}/_internal/deferred` (default **`/api/_internal/deferred`**), exposed by `register_deferred_invoke_route()` when core routes are registered (`AppBuilder.register_core_routes`). If you assemble a `FastAPI` app without that path, call `jvspatial.api.deferred_invoke_route.register_deferred_invoke_route(app)` yourself. - **Disable**: set `JVSPATIAL_DEFERRED_INVOKE_DISABLED=true` to skip registering the route (e.g. when another entrypoint handles deferred work). - **Dispatch**: the body must be a JSON object with a string **`task_type`**. jvspatial dispatches to handlers registered via **`register_deferred_invoke_handler(task_type, fn)`** or **`@deferred_invoke_handler("…")`** (also exported from top-level **`jvspatial`**). Unknown `task_type` yields HTTP 404. -- **Auth**: The deferred HTTP path is **always exempt from `AuthenticationMiddleware`** (JWT/API key): Lambda async invoke bodies cannot carry your app’s `Authorization` header. Inside the deferred route: **loopback peers** (`127.0.0.1` / `::1` / `localhost` — LWA pass-through self-invoke) are always allowed. **Non-loopback** callers (Function URL / API Gateway) require **`JVSPATIAL_DEFERRED_INVOKE_SECRET`** via **`X-JVSPATIAL-Deferred-Authorize`** or **`Authorization: Bearer `**; if the secret is unset, those callers get 401 (fail-closed). Still prefer private network / VPC boundaries for production. +- **Auth**: The deferred HTTP path is exempt from `AuthenticationMiddleware` (JWT/API key). The deferred route requires `JVSPATIAL_DEFERRED_INVOKE_SECRET` from every peer via `X-JVSPATIAL-Deferred-Authorize` or `Authorization: Bearer `. Lambda Web Adapter self-invoke can explicitly allow loopback (`127.0.0.1`, `::1`, `localhost`) without a header by setting `JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK=true`. An unset secret otherwise returns 401. Keep the route behind private network boundaries in production. - **LWA environment (best-effort)**: when `is_serverless_mode()` is true, `detect_serverless_provider() == "aws"`, and LWA is detected (e.g. `AWS_LWA_PORT` or `AWS_LAMBDA_EXEC_WRAPPER` indicating the adapter), **`apply_aws_lwa_env_defaults()`** (`jvspatial.runtime.lwa`) runs from **`Server.__init__`** and uses `os.environ.setdefault` for **`AWS_LWA_PASS_THROUGH_PATH`** (same path rule as `{JVSPATIAL_API_PREFIX}/_internal/deferred`) and **`AWS_LWA_INVOKE_MODE=RESPONSE_STREAM`**. Set **`JVSPATIAL_LWA_ENV_DEFAULTS=true`** to force these defaults if detection misses; **`JVSPATIAL_LWA_ENV_DEFAULTS=false`** to disable. The LWA extension may still read env before Python starts, so **set them in Lambda / IaC** when you need guarantees. - **EventBridge default (best-effort)**: when `is_serverless_mode()` is true and `detect_serverless_provider() == "aws"`, **`apply_aws_eventbridge_env_default()`** (`jvspatial.runtime.lwa`) runs from **`Server.__init__`**. If **`JVSPATIAL_EVENTBRIDGE_SCHEDULER_ENABLED`** is absent, it sets **`true`** or **`false`** based on whether EventBridge prerequisites are satisfied (see `jvspatial.runtime.eventbridge_readiness`). Provide **`JVSPATIAL_EVENTBRIDGE_ROLE_ARN`** and either **`JVSPATIAL_EVENTBRIDGE_LAMBDA_ARN`** or **`AWS_LAMBDA_FUNCTION_NAME`** + **`AWS_ACCOUNT_ID`** (+ region) in IaC when you want scheduler-backed `run_at`. diff --git a/docs/md/webhook-architecture.md b/docs/md/webhook-architecture.md index b5ed857..24d8182 100644 --- a/docs/md/webhook-architecture.md +++ b/docs/md/webhook-architecture.md @@ -1,5 +1,7 @@ # JVspatial Webhook Architecture Specification +> Historical design proposal. The sections below contain proposed behavior and are not the current runtime contract. See `SPEC.md` §9.5 and `jvspatial/api/integrations/webhooks/middleware.py` for implemented behavior. Current behavior: `@endpoint(webhook=True)` defaults to POST; `signature_required=True` checks HMAC on every method, including GET; idempotency database errors return 503; and handlers run within the request. The `async_processing` setting no longer starts an in-process background task. Enqueue durable work in an external queue from the handler. + ## Overview This document outlines the architecture for the webhook endpoint system in jvspatial built around the unified `@endpoint` decorator with `webhook=True`. This decorator automatically detects whether it's decorating a function or Walker class, extending the existing `@endpoint` pattern to support webhook-specific functionality while maintaining consistency in registration, metadata-driven authentication, and server integration. @@ -261,4 +263,4 @@ To address the flexibility gap while preserving simplicity/security, leverage th These adjustments add minimal overhead (path parsing in middleware) while enabling URL-driven flexibility for arbitrary payloads/external systems using existing API keys. HMAC optional for integrity; raw default enhances uncontrolled handling. Simplicity/security preserved by reuse. Test via QUICKSTART.md examples (e.g., webhook with path-embedded key). -Estimated Impact: Low (leverages existing auth); improves webhook usability for uncontrolled integrations. \ No newline at end of file +Estimated Impact: Low (leverages existing auth); improves webhook usability for uncontrolled integrations. diff --git a/docs/md/webhooks-quickstart.md b/docs/md/webhooks-quickstart.md index a410297..3e42f88 100644 --- a/docs/md/webhooks-quickstart.md +++ b/docs/md/webhooks-quickstart.md @@ -1,6 +1,6 @@ # JVspatial Webhooks Quickstart -JVspatial provides powerful webhook functionality through the `@endpoint` decorator with `webhook=True`, enabling secure, reliable webhook processing with features like HMAC verification, idempotency handling, and asynchronous processing. +JVspatial provides webhook functionality through the `@endpoint` decorator with `webhook=True`, including HMAC verification and durable idempotency claims. Handlers run within the request. Use an external queue for durable asynchronous work. ## Simplified Response Handling @@ -165,7 +165,7 @@ async def stripe_webhook(raw_body: bytes, content_type: str, endpoint): ) async def order_webhook(payload: dict, endpoint): """Webhook with idempotency protection against duplicate requests.""" - # Duplicate requests (same idempotency key) return cached response + # A completed retry with the same key and payload replays the original response. order_id = payload.get("order_id") # Process order... @@ -176,24 +176,23 @@ async def order_webhook(payload: dict, endpoint): ) ``` +Provide an `X-Idempotency-Key` (or `Idempotency-Key`) header to use this protection. A second request while the first is unfinished receives HTTP 409; the handler is not run again. Reusing a key with a different method, path, or body also receives 409. If the process fails after claiming a key, the claim remains pending because external effects may already have happened. Reconcile the operation before clearing the claim or retrying with a new key. Keep the shared database available; claim and outcome writes fail closed when it is unavailable. The response cache is limited to the configured maximum payload size. + ### Asynchronous Processing ```python @endpoint( "/webhook/bulk-process", webhook=True, - async_processing=True, permissions=["process_bulk_data"] ) async def bulk_processing_webhook(payload: dict, endpoint): - """Webhook that processes data asynchronously.""" - # This returns immediately with HTTP 200 - # Actual processing happens in background + """Enqueue durable work before acknowledging the webhook.""" batch_id = payload.get("batch_id") records = payload.get("records", []) - # Process large batch of records... + # Await an external queue's durable enqueue operation here. return endpoint.success( message="Batch processing initiated", data={ @@ -475,4 +474,3 @@ async def modern_webhook(payload: dict, endpoint): data={"status": "processed"} ) ``` - diff --git a/jvspatial/api/README.md b/jvspatial/api/README.md index 86bd83f..5dc1133 100644 --- a/jvspatial/api/README.md +++ b/jvspatial/api/README.md @@ -57,6 +57,7 @@ api/ - **CSP is strict on app routes, relaxed only on `/docs`, `/redoc`, `/openapi.json`.** (`components/app_builder.py`) - **`JVSPATIAL_DOCS_DISABLED` removes the entire docs surface.** No spec leak when truthy. (`components/app_builder.py`) - **Sessions and rate-limit counters are per-process.** Multi-worker deployments multiply configured limits by worker count. +- **Auth entrypoint limits are independent of the global limiter.** Register, login, forgot-password, and reset-password retain a 5/60s per-IP cap when auth is enabled. `RateLimitConfig(auth_entrypoint_rate_limit_enabled=False)` is for tests or a trusted replacement limiter. - **Endpoint registration is deferred.** `@endpoint` collects targets at import; `Server` resolves them at app build time. ## Modification patterns diff --git a/jvspatial/api/auth/_session_store.py b/jvspatial/api/auth/_session_store.py index ca19052..657b059 100644 --- a/jvspatial/api/auth/_session_store.py +++ b/jvspatial/api/auth/_session_store.py @@ -123,16 +123,16 @@ def _k(self, key: str) -> str: return f"{self._prefix}{key}" async def get(self, key: str) -> Optional[Any]: - """Return value for ``key`` from Redis or ``None`` on miss/error.""" + """Return value for ``key`` from Redis or ``None`` on miss.""" try: raw = await self._cache.get(self._k(key)) except Exception as exc: # pragma: no cover - depends on live Redis logger.warning( - "RedisSessionStore.get(%s) failed (%s); treating as miss", + "RedisSessionStore.get(%s) failed (%s)", key, exc, ) - return None + raise if raw is None: return None # The RedisCache layer already JSON-encodes values on set; on @@ -159,12 +159,18 @@ async def set(self, key: str, value: Any, ttl: Optional[int] = None) -> None: json.dumps(value, default=str), ttl=effective_ttl if effective_ttl > 0 else None, ) + # RedisCache currently logs and swallows transport errors. Verify + # the write so revocation cannot report success on that path. + stored = await self.get(key) + if stored != value: + raise RuntimeError("Session state write was not confirmed") except Exception as exc: # pragma: no cover - depends on live Redis logger.warning( "RedisSessionStore.set(%s) failed (%s); state will diverge", key, exc, ) + raise async def delete(self, key: str) -> None: """Remove ``key`` from Redis if present.""" diff --git a/jvspatial/api/auth/api_key_service.py b/jvspatial/api/auth/api_key_service.py index 8d10653..9b1ac7d 100644 --- a/jvspatial/api/auth/api_key_service.py +++ b/jvspatial/api/auth/api_key_service.py @@ -25,7 +25,7 @@ def __init__(self, context: Optional[GraphContext] = None): context: GraphContext instance for database operations. If None, defaults to the prime database — never the application default context. Auth state must live on - the prime DB (SPEC §9 / CLAUDE.md §1; audit §4.4). + the prime DB (SPEC §9 / AGENTS.md §1; audit §4.4). """ if context is None: from jvspatial.db import get_prime_database diff --git a/jvspatial/api/auth/models.py b/jvspatial/api/auth/models.py index 069ff12..f7d7c1f 100644 --- a/jvspatial/api/auth/models.py +++ b/jvspatial/api/auth/models.py @@ -13,12 +13,12 @@ class UserCreate(BaseModel): Password complexity enforcement (uppercase, lowercase, digits, special characters, common-password denylist) is left to the application layer. - Only minimum length (6) is enforced at the framework level. + Only minimum length (12) is enforced at the framework level. """ email: EmailStr = Field(..., description="User email address") password: str = Field( - ..., min_length=6, description="User password (min 6 characters)" + ..., min_length=12, description="User password (min 12 characters)" ) @@ -27,7 +27,7 @@ class UserCreateAdmin(BaseModel): email: EmailStr = Field(..., description="User email address") password: str = Field( - ..., min_length=6, description="User password (min 6 characters)" + ..., min_length=12, description="User password (min 12 characters)" ) name: str = Field( default="", @@ -106,7 +106,7 @@ class PasswordChangeRequest(BaseModel): current_password: str = Field(..., description="Current password for verification") new_password: str = Field( - ..., min_length=6, description="New password (min 6 characters)" + ..., min_length=12, description="New password (min 12 characters)" ) @@ -121,7 +121,7 @@ class ResetPasswordRequest(BaseModel): token: str = Field(..., description="Password reset token from email") new_password: str = Field( - ..., min_length=6, description="New password (min 6 characters)" + ..., min_length=12, description="New password (min 12 characters)" ) diff --git a/jvspatial/api/auth/oauth/keys.py b/jvspatial/api/auth/oauth/keys.py index 3684fc3..0ae8044 100644 --- a/jvspatial/api/auth/oauth/keys.py +++ b/jvspatial/api/auth/oauth/keys.py @@ -6,15 +6,55 @@ from __future__ import annotations +import os import uuid from typing import Any, Dict, List, Optional, Tuple, cast import jwt # PyJWT +from cryptography.fernet import Fernet, InvalidToken from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import rsa from jvspatial.api.auth.oauth.models import OAuthSigningKey +_ENCRYPTED_PREFIX = "fernet:v1:" +_ENCRYPTION_ENV = "JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY" + + +def _cipher() -> Optional[Fernet]: + value = os.environ.get(_ENCRYPTION_ENV) + if not value: + if os.environ.get("JVSPATIAL_ENVIRONMENT", "").lower() == "production": + raise RuntimeError( + f"{_ENCRYPTION_ENV} is required for production OAuth signing keys" + ) + return None + try: + return Fernet(value.encode("ascii")) + except ValueError as exc: + raise RuntimeError(f"{_ENCRYPTION_ENV} is not a valid Fernet key") from exc + + +async def _read_private_key(key: OAuthSigningKey) -> OAuthSigningKey: + """Return a signing copy with plaintext PEM; keep the persisted row encrypted.""" + cipher = _cipher() + stored = key.private_pem + if stored.startswith(_ENCRYPTED_PREFIX): + if cipher is None: + raise RuntimeError( + f"{_ENCRYPTION_ENV} is required to read OAuth signing keys" + ) + try: + plaintext = cipher.decrypt(stored[len(_ENCRYPTED_PREFIX) :].encode("ascii")) + except (InvalidToken, ValueError) as exc: + raise RuntimeError("OAuth signing key decryption failed") from exc + return key.model_copy(update={"private_pem": plaintext.decode("utf-8")}) + if cipher is None: + return key + # Existing plaintext rows are rewrapped on first use after enabling encryption. + await key.save() + return key.model_copy(update={"private_pem": stored}) + def _generate_rsa_pem_pair() -> Tuple[str, str]: """Return ``(private_pem, public_pem)`` for a fresh RSA-2048 keypair.""" @@ -46,7 +86,7 @@ async def generate_signing_key() -> OAuthSigningKey: active=True, ) await key.save() - return key + return key.model_copy(update={"private_pem": private_pem}) async def get_active_signing_key() -> Optional[OAuthSigningKey]: @@ -58,7 +98,9 @@ async def get_active_signing_key() -> Optional[OAuthSigningKey]: ) if not active: return None - return sorted(active, key=lambda k: k.created_at, reverse=True)[0] + return await _read_private_key( + sorted(active, key=lambda k: k.created_at, reverse=True)[0] + ) async def ensure_signing_key() -> OAuthSigningKey: diff --git a/jvspatial/api/auth/oauth/models.py b/jvspatial/api/auth/oauth/models.py index bb99378..af84859 100644 --- a/jvspatial/api/auth/oauth/models.py +++ b/jvspatial/api/auth/oauth/models.py @@ -93,13 +93,13 @@ class OAuthSigningKey(Object): """A persisted RS256 signing keypair. Active keys sign new tokens; inactive-but-recent keys remain in JWKS for the - verification window (rotation). Private PEM is stored as-is here; production - deployments should wrap it (env/KMS) — see plan assumptions. + verification window (rotation). The keystore encrypts private PEM before + persistence when ``JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY`` is configured. """ kid: str = Field(..., description="Key ID (JWKS 'kid')") public_pem: str = Field(..., description="PEM-encoded public key") - private_pem: str = Field(..., description="PEM-encoded private key") + private_pem: str = Field(..., description="Encrypted private key or legacy PEM") algorithm: str = Field(default="RS256", description="Signing algorithm") active: bool = Field(default=True, description="Whether this key signs new tokens") created_at: datetime = Field( @@ -107,6 +107,18 @@ class OAuthSigningKey(Object): description="Creation timestamp", ) + async def save(self) -> "OAuthSigningKey": + """Never persist plaintext when key encryption is configured.""" + from jvspatial.api.auth.oauth.keys import _ENCRYPTED_PREFIX, _cipher + + cipher = _cipher() + if cipher is not None and not self.private_pem.startswith(_ENCRYPTED_PREFIX): + self.private_pem = _ENCRYPTED_PREFIX + cipher.encrypt( + self.private_pem.encode("utf-8") + ).decode("ascii") + await super().save() + return self + class OAuthRevokedToken(Object): """A denylisted access-token ``jti`` (RFC 7009 access-token revocation). diff --git a/jvspatial/api/auth/service.py b/jvspatial/api/auth/service.py index 698b35f..e89f2e6 100644 --- a/jvspatial/api/auth/service.py +++ b/jvspatial/api/auth/service.py @@ -5,7 +5,6 @@ import hmac import logging import secrets -import time import uuid from datetime import datetime, timedelta, timezone from typing import Any, Callable, Dict, List, Optional, Tuple @@ -145,7 +144,7 @@ def __init__( blacklist_fail_closed if blacklist_fail_closed is not None else env( - "JVSPATIAL_AUTH_BLACKLIST_FAIL_CLOSED", default=False, parse=parse_bool + "JVSPATIAL_AUTH_BLACKLIST_FAIL_CLOSED", default=True, parse=parse_bool ) ) self.password_reset_token_expiry_minutes = ( @@ -336,7 +335,7 @@ def _hash_refresh_token(self, token: str) -> str: elif _HASHING_LIB == "passlib": return _passlib_context.hash(token) else: - if env("JVSPATIAL_AUTH_STRICT_HASHING", default=False, parse=parse_bool): + if env("JVSPATIAL_AUTH_STRICT_HASHING", default=True, parse=parse_bool): raise RuntimeError( "Secure hashing library required but unavailable. " "Install bcrypt/argon2/passlib or disable JVSPATIAL_AUTH_STRICT_HASHING." @@ -383,7 +382,7 @@ def _verify_refresh_token(self, token: str, hashed: str) -> bool: return False else: # Constant-time comparison is non-negotiable for any secret - # comparison (SPEC §15.2, CLAUDE.md invariant §2). The earlier + # comparison (SPEC §15.2, AGENTS.md invariant §2). The earlier # ``==`` form was timing-leakable on partial hex prefix matches. return hmac.compare_digest( hashlib.sha256(token.encode()).hexdigest(), hashed @@ -596,6 +595,7 @@ async def _blacklist_token(self, token: str) -> bool: return True except Exception: + self._logger.exception("Failed to blacklist access token") return False def _generate_refresh_token_string(self) -> str: @@ -777,9 +777,8 @@ async def register_user(self, user_data: UserCreate) -> UserResponse: if not self.registration_open: raise RegistrationDisabledError() - # Bootstrap: first user gets admin role; others get default role - user_count = await self._user_count() - initial_roles = [self.admin_role] if user_count == 0 else [self.default_role] + # Public registration never grants privileges based on a racy user count. + initial_roles = [self.default_role] # Create new user from jvspatial.core.utils import generate_id @@ -824,14 +823,14 @@ async def bootstrap_admin( Args: email: Admin email address - password: Admin password (min 6 characters) + password: Admin password (min 12 characters) name: Optional display name (defaults to email) Returns: UserResponse if admin was created, None if admin already exists """ - if len(password) < 6: - raise ValueError("Password must be at least 6 characters") + if len(password) < 12: + raise ValueError("Password must be at least 12 characters") existing = await self._find_user_by_email(email) if existing: @@ -850,7 +849,10 @@ async def bootstrap_admin( if user and self.admin_role in (user.roles or []): return None except Exception: - continue + self._logger.exception( + "Unable to inspect existing user during admin bootstrap" + ) + raise from jvspatial.core.utils import generate_id @@ -919,7 +921,7 @@ async def login_user( # Check if user is active if not user.is_active: - raise ValueError("User account is deactivated") + raise ValueError("Invalid email or password") # Update last_accessed timestamp and save (includes password migration if applied) user._graph_context = self.context @@ -982,7 +984,13 @@ async def logout_user(self, token: str) -> bool: Returns: True if successfully logged out, False otherwise """ - return await self._blacklist_token(token) + payload = self._decode_jwt_token(token) + if not payload or not await self._blacklist_token(token): + return False + jti = payload.get("jti") + if jti: + await self._deactivate_refresh_tokens(payload.get("user_id", ""), jti) + return True async def validate_token(self, token: str) -> Optional[UserResponse]: """Validate a JWT token and return user information. @@ -1022,17 +1030,11 @@ async def validate_token(self, token: str) -> Optional[UserResponse]: # Find user by ID using service's context user = None - got_db_error = False try: user = await self._get_user_by_id(user_id) - except Exception as e: - from jvspatial.exceptions import DatabaseError - - # Only enter JWT-payload fallback for unambiguous DB errors - # (connection failures, path mismatches), not for generic exceptions. - if isinstance(e, DatabaseError): - got_db_error = True - self._logger.warning("[validate_token] _get_user_by_id error: %s", e) + except Exception: + self._logger.exception("[validate_token] user lookup failed") + return None # Fallback: lookup by email when get-by-id fails (e.g. context/db path mismatch). if not user: @@ -1043,7 +1045,8 @@ async def validate_token(self, token: str) -> Optional[UserResponse]: if user and user.id != user_id: user = None except Exception: - pass + self._logger.exception("[validate_token] email lookup failed") + return None # Fallback: direct db.find by id (bypasses context.get which may fail) if not user: @@ -1056,20 +1059,8 @@ async def validate_token(self, token: str) -> Optional[UserResponse]: if user: user._graph_context = self.context except Exception: - pass - - # Fallback: build UserResponse from payload only when DB raised an error (e.g. path - # mismatch). When user simply doesn't exist (None), return None for security. - if not user and user_id and got_db_error: - return UserResponse( - id=user_id, - email=payload.get("email", ""), - name=payload.get("name", ""), - created_at=datetime.now(timezone.utc), - is_active=True, - roles=payload.get("roles") or [self.default_role], - permissions=list(payload.get("permissions") or []), - ) + self._logger.exception("[validate_token] direct lookup failed") + return None if not user: # Avoid logging the filesystem path of the database — leaks @@ -1089,12 +1080,8 @@ async def validate_token(self, token: str) -> Optional[UserResponse]: ) return None - # Use roles/permissions from JWT payload if present, else compute from user - roles = payload.get("roles") - permissions = payload.get("permissions") - if roles is None or permissions is None: - roles = self._get_user_roles(user) - permissions = self._get_effective_permissions_for_user(user) + roles = self._get_user_roles(user) + permissions = self._get_effective_permissions_for_user(user) # Update last_accessed timestamp on token validation (user is authenticating). # Non-blocking: do not fail auth if save fails (e.g. SQLite lock under concurrent load). @@ -1238,7 +1225,7 @@ async def revoke_refresh_token(self, refresh_token: str) -> bool: refresh_token_entity._graph_context = self.context await self.context.save(refresh_token_entity) - # Optionally blacklist the associated access token + # Blacklist the associated access token before reporting success. if refresh_token_entity.access_token_jti: # Try to blacklist the access token (may fail if already expired) # We can't decode it without the token string, but we can add to blacklist by JTI @@ -1260,12 +1247,16 @@ async def revoke_refresh_token(self, refresh_token: str) -> bool: blacklist_entry._graph_context = self.context try: await self.context.save(blacklist_entry) - # Update cache - cache_key = f"blacklist:{refresh_token_entity.access_token_jti}" - self._blacklist_cache[cache_key] = (True, time.time()) + await self._session_store.set( + f"blacklist:{refresh_token_entity.access_token_jti}", + True, + ttl=self.blacklist_cache_ttl_seconds, + ) except Exception: - # Ignore errors - token may already be expired or blacklisted - pass + self._logger.exception( + "Failed to blacklist access token during refresh revocation" + ) + raise return True @@ -1289,20 +1280,17 @@ async def revoke_all_user_tokens(self, user_id: str) -> int: access_token_jtis = [] for data in results: - try: - refresh_token_entity = await self.context._deserialize_entity( - RefreshToken, data - ) - if refresh_token_entity: - refresh_token_entity._graph_context = self.context - refresh_token_entity.is_active = False - await self.context.save(refresh_token_entity) - revoked_count += 1 + refresh_token_entity = await self.context._deserialize_entity( + RefreshToken, data + ) + if refresh_token_entity: + refresh_token_entity._graph_context = self.context + refresh_token_entity.is_active = False + await self.context.save(refresh_token_entity) + revoked_count += 1 - if refresh_token_entity.access_token_jti: - access_token_jtis.append(refresh_token_entity.access_token_jti) - except Exception: - continue + if refresh_token_entity.access_token_jti: + access_token_jtis.append(refresh_token_entity.access_token_jti) # Blacklist all associated access tokens from jvspatial.core.utils import generate_id @@ -1312,26 +1300,46 @@ async def revoke_all_user_tokens(self, user_id: str) -> int: ) for jti in access_token_jtis: + blacklist_id = generate_id("o", "TokenBlacklist") + blacklist_entry = TokenBlacklist( + id=blacklist_id, + token_id=jti, + user_id=user_id, + expires_at=estimated_expires_at, + ) + blacklist_entry._graph_context = self.context try: - blacklist_id = generate_id("o", "TokenBlacklist") - blacklist_entry = TokenBlacklist( - id=blacklist_id, - token_id=jti, - user_id=user_id, - expires_at=estimated_expires_at, - ) - blacklist_entry._graph_context = self.context await self.context.save(blacklist_entry) - - # Update cache - cache_key = f"blacklist:{jti}" - self._blacklist_cache[cache_key] = (True, time.time()) + await self._session_store.set( + f"blacklist:{jti}", True, ttl=self.blacklist_cache_ttl_seconds + ) except Exception: - # Ignore errors - token may already be expired or blacklisted - pass + self._logger.exception( + "Failed to blacklist access token during bulk revocation" + ) + raise return revoked_count + async def _deactivate_refresh_tokens(self, user_id: str, access_jti: str) -> None: + """Deactivate refresh tokens bound to one access-token JTI.""" + await self.context.ensure_indexes(RefreshToken) + collection, query = await RefreshToken._build_database_query( + self.context, + { + "context.user_id": user_id, + "context.access_token_jti": access_jti, + "context.is_active": True, + }, + {}, + ) + for data in await self.context.database.find(collection, query): + entity = await self.context._deserialize_entity(RefreshToken, data) + if entity: + entity._graph_context = self.context + entity.is_active = False + await self.context.save(entity) + async def change_password( self, user_id: str, current_password: str, new_password: str ) -> bool: @@ -1340,7 +1348,7 @@ async def change_password( Args: user_id: User ID current_password: Current password for verification - new_password: New password (min 6 characters) + new_password: New password (min 12 characters) Returns: True on success @@ -1355,6 +1363,8 @@ async def change_password( if not self._verify_password(current_password, user.password_hash): raise ValueError("Current password is incorrect") + if len(new_password) < 12: + raise ValueError("Password must be at least 12 characters") user.password_hash = self._hash_password(new_password) user._graph_context = self.context await self.context.save(user) @@ -1431,7 +1441,7 @@ async def reset_password_with_token(self, token: str, new_password: str) -> bool Args: token: Plaintext reset token from email - new_password: New password (min 6 characters) + new_password: New password (min 12 characters) Returns: True on success @@ -1468,18 +1478,28 @@ async def reset_password_with_token(self, token: str, new_password: str) -> bool if not user: raise ValueError("Invalid or expired token") - user.password_hash = self._hash_password(new_password) - await self.context.save(user) - + if len(new_password) < 12: + raise ValueError("Password must be at least 12 characters") entity.used_at = now await self.context.save(entity) + user.password_hash = self._hash_password(new_password) + await self.context.save(user) - await self.revoke_all_user_tokens(entity.user_id) + try: + await self.revoke_all_user_tokens(entity.user_id) + except Exception: + self._logger.exception( + "Password reset succeeded but token revocation failed" + ) + raise RuntimeError( + "Password reset completed; token revocation failed" + ) return True except ValueError: raise except Exception: - continue + self._logger.exception("Password reset failed") + raise raise ValueError("Invalid or expired token") @@ -1549,6 +1569,7 @@ async def update_user_roles( user.roles = roles_update.roles user._graph_context = self.context await self.context.save(user) + await self.revoke_all_user_tokens(user_id) return await self.get_user_by_id(user_id) @@ -1571,6 +1592,7 @@ async def update_user_permissions( user.permissions = permissions_update.permissions user._graph_context = self.context await self.context.save(user) + await self.revoke_all_user_tokens(user_id) return await self.get_user_by_id(user_id) diff --git a/jvspatial/api/components/app_builder.py b/jvspatial/api/components/app_builder.py index b4ad888..122d542 100644 --- a/jvspatial/api/components/app_builder.py +++ b/jvspatial/api/components/app_builder.py @@ -59,8 +59,12 @@ def create_app(self, lifespan: Optional[Any] = None) -> FastAPI: # (audit §7.2 / §7.12). from jvspatial.env import env, parse_bool + production = ( + str(env("JVSPATIAL_ENVIRONMENT", default="development")).lower() + == "production" + ) docs_disabled = bool( - env("JVSPATIAL_DOCS_DISABLED", default=False, parse=parse_bool) + env("JVSPATIAL_DOCS_DISABLED", default=production, parse=parse_bool) ) app_kwargs: Dict[str, Any] = { diff --git a/jvspatial/api/components/auth_configurator.py b/jvspatial/api/components/auth_configurator.py index e070891..c91fd67 100644 --- a/jvspatial/api/components/auth_configurator.py +++ b/jvspatial/api/components/auth_configurator.py @@ -199,10 +199,10 @@ async def register(request: Request): status_code=422, detail="email and password are required", ) - if len(password) < 6: + if len(password) < 12: raise HTTPException( status_code=422, - detail="password must be at least 6 characters", + detail="password must be at least 12 characters", ) try: auth_service = get_auth_service() @@ -226,8 +226,8 @@ async def register(request: Request): return user except RegistrationDisabledError: raise - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) + except ValueError: + raise HTTPException(status_code=400, detail="Registration unavailable") except Exception as e: self._logger.error(f"Registration error: {e}") raise HTTPException(status_code=500, detail="Internal server error") @@ -267,8 +267,8 @@ async def login(login_data: UserLogin): auth_service = get_auth_service() token_response = await auth_service.login_user(login_data) return token_response - except ValueError as e: - raise HTTPException(status_code=401, detail=str(e)) + except ValueError: + raise HTTPException(status_code=401, detail="Invalid email or password") except Exception as e: self._logger.error(f"Login error: {e}") raise HTTPException(status_code=500, detail="Internal server error") diff --git a/jvspatial/api/components/auth_middleware.py b/jvspatial/api/components/auth_middleware.py index 5fbb6c5..9418d2e 100644 --- a/jvspatial/api/components/auth_middleware.py +++ b/jvspatial/api/components/auth_middleware.py @@ -152,10 +152,19 @@ async def dispatch(self, request: Request, call_next): # RBAC: check roles and permissions if endpoint requires them rbac_enabled = getattr(self.auth_config, "rbac_enabled", True) if rbac_enabled: - endpoint_config = self._auth_resolver.get_endpoint_config(request) + try: + endpoint_config = self._auth_resolver.get_endpoint_config(request) + except Exception: + self._logger.exception("Endpoint auth configuration unavailable") + return JSONResponse(status_code=403, content={"message": "Forbidden"}) endpoint_config = self._merge_admin_only_roles( request.url.path, endpoint_config ) + # A successful lookup can legitimately find no jvspatial endpoint + # metadata on a mounted ASGI app or a raw FastAPI route. The user + # has still been authenticated above. Admin-only paths acquire a + # mandatory role from _merge_admin_only_roles even in that case. + # Lookup failures themselves remain fail-closed in the except arm. rbac_error = self._check_rbac(user, endpoint_config) if rbac_error: return rbac_error @@ -461,11 +470,13 @@ async def _authenticate_api_key(self, request: Request) -> Optional[Any]: request_path = request.url.path def _endpoint_allowed(ep: str) -> bool: - # Support wildcard: /api/webhook/* matches /api/webhook/xyz + # A wildcard must cover complete path segments. if ep.endswith("*"): - prefix = ep[:-1] - return request_path.startswith(prefix) - return request_path.startswith(ep) + prefix = ep[:-1].rstrip("/") + return request_path == prefix or request_path.startswith( + prefix + "/" + ) + return request_path == ep if not any( _endpoint_allowed(ep) for ep in api_key_entity.allowed_endpoints diff --git a/jvspatial/api/components/endpoint_auth_resolver.py b/jvspatial/api/components/endpoint_auth_resolver.py index 93f4791..66c1376 100644 --- a/jvspatial/api/components/endpoint_auth_resolver.py +++ b/jvspatial/api/components/endpoint_auth_resolver.py @@ -192,24 +192,6 @@ def endpoint_requires_auth(self, request: Request) -> bool: and request_method not in route.methods ): continue - if route.dependencies: - for dep in route.dependencies: - s = str(dep).lower() - # Heuristic: detect FastAPI security dependencies by known - # class/function names. More robust than substring "auth". - if any( - kw in s - for kw in ( - "httpbearer", - "httpbasic", - "httpdigest", - "oauth2passwordbearer", - "apikey", - "security", - "bearer", - ) - ): - return True if "/auth/" in request_path and not self._path_matcher.is_exempt( request_path ): @@ -251,8 +233,18 @@ def endpoint_requires_auth(self, request: Request) -> bool: return True def endpoint_has_fastapi_auth(self, request: Request) -> bool: - """Check if route has FastAPI auth dependencies.""" + """Check for an enforcing FastAPI security dependency on a raw route. + + Dependency names are not evidence of authentication. A dependency + called ``auth_context`` may simply return optional request metadata. + """ try: + from fastapi.security.base import SecurityBase + + if path_requires_admin_only_role(request.url.path): + return False + if self.get_endpoint_config(request) is not None: + return False if ( not self._server or not getattr(self._server, "app", None) @@ -286,11 +278,13 @@ def endpoint_has_fastapi_auth(self, request: Request) -> bool: and request.method not in route.methods ): continue - if route.dependencies: - for dep in route.dependencies: - s = str(dep).lower() - if "security" in s or "bearer" in s or "auth" in s: - return True + pending = list(route.dependant.dependencies) + while pending: + dependency = pending.pop() + security = dependency.call + if isinstance(security, SecurityBase) and security.auto_error: + return True + pending.extend(dependency.dependencies) return False except Exception: return False @@ -354,7 +348,8 @@ def get_endpoint_config(self, request: Request) -> Optional[Dict[str, Any]]: return route.endpoint._jvspatial_endpoint_config # type: ignore[attr-defined] return None except Exception: - return None + self._logger.exception("Endpoint auth configuration lookup failed") + raise __all__ = ["EndpointAuthResolver", "path_requires_admin_only_role"] diff --git a/jvspatial/api/components/error_handler.py b/jvspatial/api/components/error_handler.py index a3f5925..66801e7 100644 --- a/jvspatial/api/components/error_handler.py +++ b/jvspatial/api/components/error_handler.py @@ -613,7 +613,9 @@ async def handle_exception(request: Request, exc: Exception) -> JSONResponse: # Extract error message from detail - handle string, dict, list, or None # Do this before logging so we can use it in log messages - error_detail = exc.detail + error_detail = ( + "Internal server error" if exc.status_code >= 500 else exc.detail + ) if error_detail is None: error_message = "An error occurred" elif isinstance(error_detail, str): diff --git a/jvspatial/api/config_groups.py b/jvspatial/api/config_groups.py index 42474b4..24847fa 100644 --- a/jvspatial/api/config_groups.py +++ b/jvspatial/api/config_groups.py @@ -246,7 +246,7 @@ class AuthConfig(BaseModel): ) bootstrap_admin_password: Optional[str] = Field( default=None, - description="Admin password for bootstrap. Min 6 chars. Typically from ADMIN_PASSWORD env.", + description="Admin password for bootstrap. Min 12 chars. Typically from ADMIN_PASSWORD env.", ) bootstrap_admin_name: Optional[str] = Field( default=None, @@ -341,6 +341,13 @@ class RateLimitConfig(BaseModel): rate_limit_enabled: bool = Field( default=False, description="Enable rate limiting middleware" ) + auth_entrypoint_rate_limit_enabled: bool = Field( + default=True, + description=( + "Limit auth entry points even when the global limiter is disabled; " + "set false only when a trusted host limiter covers them or in tests" + ), + ) rate_limit_default_requests: int = Field( default=60, description="Default requests per window" ) diff --git a/jvspatial/api/decorators/route.py b/jvspatial/api/decorators/route.py index d8acb4b..52f64a8 100644 --- a/jvspatial/api/decorators/route.py +++ b/jvspatial/api/decorators/route.py @@ -166,7 +166,7 @@ def decorator( } config = { "path": path, - "methods": methods or ["GET"], + "methods": methods or (["POST"] if webhook else ["GET"]), "auth_required": config_auth, "permissions": config_permissions, "roles": config_roles, diff --git a/jvspatial/api/deferred_invoke_route.py b/jvspatial/api/deferred_invoke_route.py index cb4a18b..912b5fe 100644 --- a/jvspatial/api/deferred_invoke_route.py +++ b/jvspatial/api/deferred_invoke_route.py @@ -46,16 +46,18 @@ def _is_loopback_client(request: Request) -> bool: def _deferred_invoke_secret_ok(request: Request) -> bool: """Authorize the internal deferred-invoke endpoint. - Lambda Web Adapter self-invoke POSTs from loopback without auth headers, so - loopback peers are always allowed. Non-loopback callers (Function URL / - API Gateway) fail closed when ``JVSPATIAL_DEFERRED_INVOKE_SECRET`` is unset - or empty (audit §4.16 / SPEC §15.2); when set, they must send the value in + Lambda Web Adapter self-invoke POSTs from loopback without auth headers; + deployments can explicitly enable that exception with + ``JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK``. Otherwise every peer must + send ``JVSPATIAL_DEFERRED_INVOKE_SECRET`` in ``X-JVSPATIAL-Deferred-Authorize`` or ``Authorization: Bearer …``. Disable the route entirely via ``JVSPATIAL_DEFERRED_INVOKE_DISABLED=true`` if you do not need it. """ - if _is_loopback_client(request): + if _is_loopback_client(request) and env( + "JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK", default=False, parse=parse_bool + ): return True secret = env("JVSPATIAL_DEFERRED_INVOKE_SECRET") or "" @@ -79,10 +81,10 @@ def _deferred_invoke_secret_ok(request: Request) -> bool: def register_deferred_invoke_route(app: FastAPI) -> None: """Mount the internal deferred-invoke endpoint. - Loopback callers (LWA pass-through) are always authorized. Non-loopback - callers require ``JVSPATIAL_DEFERRED_INVOKE_SECRET`` via header + All callers require ``JVSPATIAL_DEFERRED_INVOKE_SECRET`` via header ``X-JVSPATIAL-Deferred-Authorize`` or ``Authorization: Bearer …``. Set ``JVSPATIAL_DEFERRED_INVOKE_DISABLED=true`` to skip registering the route. + Set ``JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK=true`` only for LWA self-invoke. """ if _deferred_invoke_disabled(): diff --git a/jvspatial/api/endpoints/graph_visualization.py b/jvspatial/api/endpoints/graph_visualization.py index 773486e..0febc3f 100644 --- a/jvspatial/api/endpoints/graph_visualization.py +++ b/jvspatial/api/endpoints/graph_visualization.py @@ -69,9 +69,7 @@ async def graph_expand( ) except Exception as e: logger.exception("graph expand failed") - raise HTTPException( - status_code=500, detail=f"Graph expand failed: {e}" - ) from e + raise HTTPException(status_code=500, detail="Internal server error") from e tag = weak_etag_for_payload(payload) return JSONResponse( content=payload, @@ -111,9 +109,7 @@ async def graph_subgraph( ) except Exception as e: logger.exception("graph subgraph failed") - raise HTTPException( - status_code=500, detail=f"Graph subgraph failed: {e}" - ) from e + raise HTTPException(status_code=500, detail="Internal server error") from e tag = weak_etag_for_payload(payload) return JSONResponse( content=payload, diff --git a/jvspatial/api/integrations/scheduler/scheduler.py b/jvspatial/api/integrations/scheduler/scheduler.py index 98d9d27..1960da3 100644 --- a/jvspatial/api/integrations/scheduler/scheduler.py +++ b/jvspatial/api/integrations/scheduler/scheduler.py @@ -69,6 +69,7 @@ def __init__( self._executor = ThreadPoolExecutor( max_workers=self.config.max_concurrent_tasks ) + self._server_loop: Optional[asyncio.AbstractEventLoop] = None # Task management self._tasks: Dict[str, ScheduledTask] = {} @@ -115,6 +116,11 @@ def start(self, interval: Optional[float] = None) -> Optional[threading.Event]: ) return None + try: + self._server_loop = asyncio.get_running_loop() + except RuntimeError as exc: + raise RuntimeError("Scheduler must start on the server event loop") from exc + if interval is None: interval = self.config.interval @@ -615,15 +621,14 @@ def _run_async_task(self, func: Callable, timeout: int) -> Any: Returns: Function result """ + if self._server_loop is None or not self._server_loop.is_running(): + raise RuntimeError("Scheduler server event loop is unavailable") + future = asyncio.run_coroutine_threadsafe(func(), self._server_loop) try: - # Create new event loop for this thread - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - - # Run with timeout - return loop.run_until_complete(asyncio.wait_for(func(), timeout=timeout)) - finally: - loop.close() + return future.result(timeout=timeout) + except Exception: + future.cancel() + raise def _run_walker_task(self, walker_class: type, timeout: int) -> Any: """Run a Walker class. @@ -635,33 +640,25 @@ def _run_walker_task(self, walker_class: type, timeout: int) -> Any: Returns: Walker execution result """ - try: - # Create new event loop for this thread - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - - async def run_walker(): - if self.graph_context: - async with self.graph_context: - walker = walker_class() - return await walker.spawn() - else: - walker = walker_class() - return await walker.spawn() + if self._server_loop is None or not self._server_loop.is_running(): + raise RuntimeError("Scheduler server event loop is unavailable") - # Run with timeout - result_walker = loop.run_until_complete( - asyncio.wait_for(run_walker(), timeout=timeout) - ) - - # Return the report as the result - if hasattr(result_walker, "get_report"): - return {"walker_report": result_walker.get_report()} - else: - return {"walker_id": result_walker.id} + async def run_walker(): + if self.graph_context: + async with self.graph_context: + walker = walker_class() + await walker.spawn() + return {"walker_report": await walker.get_report()} + walker = walker_class() + await walker.spawn() + return {"walker_report": await walker.get_report()} - finally: - loop.close() + future = asyncio.run_coroutine_threadsafe(run_walker(), self._server_loop) + try: + return future.result(timeout=timeout) + except Exception: + future.cancel() + raise def _update_execution_stats(self, execution_record: TaskExecutionRecord) -> None: """Update execution statistics. diff --git a/jvspatial/api/integrations/storage/service.py b/jvspatial/api/integrations/storage/service.py index bea61b1..beb875c 100644 --- a/jvspatial/api/integrations/storage/service.py +++ b/jvspatial/api/integrations/storage/service.py @@ -4,6 +4,7 @@ operations, separating concerns from the main Server class. """ +import logging import mimetypes from typing import Any, Dict, Optional @@ -19,6 +20,7 @@ from jvspatial.storage.exceptions import StorageError _FILES_OPENAPI_TAGS = ["Files"] +logger = logging.getLogger(__name__) def _media_type_for_path(file_path: str) -> str: @@ -65,6 +67,18 @@ def __init__( self.proxy_manager = proxy_manager self.config = config + def _proxy_public_url(self, code: str) -> str: + base_url = ( + getattr( + getattr(self.config, "file_storage", None), + "file_storage_base_url", + None, + ) + or getattr(self.file_interface, "base_url", "") + or "" + ) + return f"{base_url.rstrip('/')}{APIRoutes.PROXY_PREFIX}/{code}" + async def handle_upload( self, file: UploadFile, @@ -107,20 +121,21 @@ async def handle_upload( # Create proxy if requested if create_proxy and self.proxy_manager: - proxy_url = self.proxy_manager.create_proxy( + proxy = await self.proxy_manager.create_proxy( file_path=file_path, expires_in=proxy_expires_in, one_time=proxy_one_time, ) - result["proxy_url"] = proxy_url - result["proxy_code"] = proxy_url.split("/")[-1] + result["proxy_url"] = self._proxy_public_url(proxy.code) + result["proxy_code"] = proxy.code return result except (PathTraversalError, ValidationError) as e: raise HTTPException(status_code=400, detail=str(e)) - except StorageError as e: - raise HTTPException(status_code=500, detail=str(e)) + except StorageError: + logger.exception("File upload failed") + raise HTTPException(status_code=500, detail="Internal server error") async def handle_serve(self, file_path: str) -> Response: """Handle file serving/download. @@ -140,8 +155,9 @@ async def handle_serve(self, file_path: str) -> Response: return StreamingResponse(stream, media_type=media_type) except FileNotFoundError: raise HTTPException(status_code=404, detail=ErrorMessages.FILE_NOT_FOUND) - except StorageError as e: - raise HTTPException(status_code=500, detail=str(e)) + except StorageError: + logger.exception("File serving failed") + raise HTTPException(status_code=500, detail="Internal server error") async def handle_delete(self, file_path: str) -> Dict[str, Any]: """Handle file deletion. @@ -161,8 +177,9 @@ async def handle_delete(self, file_path: str) -> Dict[str, Any]: # ``success`` (audit §3.3). success = await self.file_interface.delete_file(file_path) return {"success": success, "file_path": file_path} - except StorageError as e: - raise HTTPException(status_code=500, detail=str(e)) + except StorageError: + logger.exception("File deletion failed") + raise HTTPException(status_code=500, detail="Internal server error") async def handle_create_proxy( self, @@ -198,7 +215,7 @@ async def handle_create_proxy( if self.proxy_manager is None: raise HTTPException(500, "Proxy manager not initialized") - proxy_url = self.proxy_manager.create_proxy( + proxy = await self.proxy_manager.create_proxy( file_path=file_path, expires_in=expires_in or default_expiration, one_time=one_time, @@ -206,13 +223,14 @@ async def handle_create_proxy( ) return { - "proxy_url": proxy_url, - "code": proxy_url.split("/")[-1], + "proxy_url": self._proxy_public_url(proxy.code), + "code": proxy.code, "file_path": file_path, "expires_in": expires_in or default_expiration, } - except StorageError as e: - raise HTTPException(500, str(e)) + except StorageError: + logger.exception("Proxy creation failed") + raise HTTPException(500, "Internal server error") async def handle_serve_proxied(self, code: str) -> Response: """Handle file serving via proxy URL. @@ -231,7 +249,7 @@ async def handle_serve_proxied(self, code: str) -> Response: raise HTTPException(500, "Proxy manager not initialized") # Resolve proxy to file path - file_path, _metadata = self.proxy_manager.resolve_proxy(code) + file_path, _metadata = await self.proxy_manager.resolve_proxy(code) stream = self.file_interface.serve_file(file_path) media_type = _media_type_for_path(file_path) @@ -239,8 +257,9 @@ async def handle_serve_proxied(self, code: str) -> Response: except FileNotFoundError: raise HTTPException(404, "Proxy not found or expired") - except StorageError as e: - raise HTTPException(500, str(e)) + except StorageError: + logger.exception("Proxy resolution failed") + raise HTTPException(500, "Internal server error") async def handle_revoke_proxy(self, code: str) -> Dict[str, Any]: """Handle proxy URL revocation. @@ -258,10 +277,11 @@ async def handle_revoke_proxy(self, code: str) -> Dict[str, Any]: if self.proxy_manager is None: raise HTTPException(500, "Proxy manager not initialized") - success = self.proxy_manager.revoke_proxy(code) + success = await self.proxy_manager.revoke_proxy(code) return {"success": success, "code": code} - except StorageError as e: - raise HTTPException(500, str(e)) + except StorageError: + logger.exception("Proxy revocation failed") + raise HTTPException(500, "Internal server error") async def handle_proxy_stats(self, code: str) -> Dict[str, Any]: """Handle proxy statistics retrieval. @@ -279,12 +299,13 @@ async def handle_proxy_stats(self, code: str) -> Dict[str, Any]: if self.proxy_manager is None: raise HTTPException(500, "Proxy manager not initialized") - stats = self.proxy_manager.get_stats(code) + stats = await self.proxy_manager.get_stats(code) if not stats: raise HTTPException(404, "Proxy not found") return dict(stats) if stats else {} - except StorageError as e: - raise HTTPException(500, str(e)) + except StorageError: + logger.exception("Proxy stats failed") + raise HTTPException(500, "Internal server error") @classmethod def register_endpoints( @@ -299,7 +320,7 @@ def register_endpoints( Routes use OpenAPI tag ``Files`` under ``{api_prefix}/files`` (from ``JVSPATIAL_API_PREFIX``). ``GET {FILES_ROOT}/{path}`` is public by default - (``JVSPATIAL_FILES_PUBLIC_READ`` true); set it false to require auth for reads. + (``JVSPATIAL_FILES_PUBLIC_READ`` true); reads require auth by default. ``POST`` upload, ``DELETE``, and proxy admin routes require auth when middleware is on. ``GET {PROXY_PREFIX}/{code}`` stays unauthenticated (proxy code is the credential). @@ -308,8 +329,13 @@ def register_endpoints( app: FastAPI application instance service: FileStorageService instance with handlers """ + auth_enabled = bool( + getattr(getattr(service.config, "auth", None), "enabled", False) + ) serve_requires_auth = not env( - "JVSPATIAL_FILES_PUBLIC_READ", default=True, parse=parse_bool_basic + "JVSPATIAL_FILES_PUBLIC_READ", + default=not auth_enabled, + parse=parse_bool_basic, ) router = APIRouter(tags=list(_FILES_OPENAPI_TAGS)) diff --git a/jvspatial/api/integrations/webhooks/middleware.py b/jvspatial/api/integrations/webhooks/middleware.py index 5c030c5..fa0e9d4 100644 --- a/jvspatial/api/integrations/webhooks/middleware.py +++ b/jvspatial/api/integrations/webhooks/middleware.py @@ -8,8 +8,7 @@ - Route parameter extraction """ -import asyncio -import json +import base64 import logging import time from typing import Any, Callable, Dict, Optional @@ -20,7 +19,6 @@ from jvspatial.api.constants import APIRoutes from jvspatial.env import normalize_optional_secret_string -from jvspatial.runtime.serverless import is_serverless_mode from .utils import ( WebhookConfig, @@ -209,63 +207,72 @@ async def dispatch(self, request: Request, call_next: Callable) -> Response: logger.debug( f"Returning cached response for duplicate request: {request.state.idempotency_key}" ) + cached = cached_response.get("_jvspatial_webhook_response_v1") + if cached: + return Response( + content=base64.b64decode(cached["body_b64"]), + status_code=cached["status_code"], + media_type=cached["content_type"], + ) return JSONResponse(content=cached_response, status_code=200) - # Handle async processing if enabled and background tasks allowed - serverless_mode = is_serverless_mode(getattr(self.server, "config", None)) - if ( - webhook_config - and webhook_config.get("async_processing", False) - and not serverless_mode - ): - # Queue for async processing and return immediate response - task_id = self._queue_async_processing(request, call_next) - from datetime import datetime - - response_data = { - "status": "queued", - "timestamp": datetime.now().isoformat(), - "message": "Webhook queued for asynchronous processing", - "task_id": task_id, - } - return JSONResponse(content=response_data, status_code=200) - - # Continue to the endpoint handler + # The handler must complete while the ASGI request is alive. For + # durable asynchronous work, handlers should enqueue externally. response = await call_next(request) # Store response for idempotency if needed idempotency_key = getattr(request.state, "idempotency_key", None) - if idempotency_key and isinstance(response, JSONResponse): + if idempotency_key: try: - response_content = ( - response.body.decode("utf-8") if response.body else "{}" + body = getattr(response, "body", None) + if body is None: + chunks = [] + total_size = 0 + async for chunk in response.body_iterator: + total_size += len(chunk) + if total_size > self.config.max_payload_size: + raise ValueError( + "Webhook response exceeds idempotency cache limit" + ) + chunks.append(chunk) + body = b"".join(chunks) + + async def replay_body(): + yield body + + response.body_iterator = replay_body() + response_data = { + "_jvspatial_webhook_response_v1": { + "body_b64": base64.b64encode(body).decode("ascii"), + "status_code": response.status_code, + "content_type": response.headers.get("content-type"), + } + } + await store_idempotent_response( + idempotency_key, response_data, status_code=response.status_code ) - response_data = json.loads(response_content) - if not serverless_mode: - asyncio.create_task( - store_idempotent_response(idempotency_key, response_data) - ) - else: - await store_idempotent_response(idempotency_key, response_data) except Exception as e: - logger.warning(f"Failed to cache response for idempotency: {e}") + logger.exception("Failed to persist webhook idempotency outcome") + raise HTTPException( + status_code=503, detail="Webhook outcome pending reconciliation" + ) from e return response except HTTPException as e: - # Convert HTTPException to proper webhook response + # Preserve authentication and availability failures for callers. logger.warning(f"Webhook processing failed: {e.detail}") from datetime import datetime error_response = { "status": "error", "timestamp": datetime.now().isoformat(), - "message": e.detail, + "message": ( + e.detail if e.status_code < 500 else "Internal processing error" + ), "error_code": e.status_code, } - return JSONResponse( - content=error_response, status_code=200 - ) # Always return 200 for webhooks + return JSONResponse(content=error_response, status_code=e.status_code) except Exception as e: # Handle unexpected errors @@ -278,9 +285,7 @@ async def dispatch(self, request: Request, call_next: Callable) -> Response: "message": "Internal processing error", "error_code": 500, } - return JSONResponse( - content=error_response, status_code=200 - ) # Always return 200 for webhooks + return JSONResponse(content=error_response, status_code=500) def _is_webhook_request(self, request: Request) -> bool: """Check if request is a webhook request. @@ -355,18 +360,32 @@ async def _process_webhook_request( ) if request.method == "GET": - # Light path for GET: no body, HMAC, or idempotency + # Sign the raw query string so its parameters cannot be changed. if config.https_required and request.url.scheme != "https": raise HTTPException( status_code=400, detail="HTTPS required for webhook endpoints", ) + if webhook_config and webhook_config.get("signature_required"): + from .utils import extract_hmac_signature, verify_hmac_signature + + signature = extract_hmac_signature(request) + if not signature or not verify_hmac_signature( + request.scope.get("query_string", b""), + signature, + config.hmac_secret or "", + ): + raise HTTPException( + status_code=401, detail="Invalid HMAC signature" + ) request.state.raw_body = b"" request.state.content_type = "" request.state.parsed_payload = dict(request.query_params) request.state.idempotency_key = None request.state.is_duplicate_request = False - request.state.hmac_verified = False + request.state.hmac_verified = bool( + webhook_config and webhook_config.get("signature_required") + ) request.state.webhook_route = self._extract_route_parameter(request) else: # Full path for POST/PUT/PATCH @@ -388,7 +407,7 @@ async def _process_webhook_request( request.state.parsed_payload = processed_data.get("parsed_payload") request.state.idempotency_key = processed_data.get("idempotency_key") request.state.is_duplicate_request = processed_data["is_duplicate"] - request.state.hmac_verified = processed_data["hmac_verified"] + request.state.hmac_verified = processed_data.get("hmac_verified", False) # Set cached response for duplicates if cached_response: @@ -481,41 +500,6 @@ def _create_processing_config( allowed_content_types=base_config.allowed_content_types, ) - async def _queue_async_processing( - self, request: Request, call_next: Callable - ) -> str: - """Queue webhook for asynchronous processing. - - Args: - request: FastAPI request object - call_next: Next handler in chain - - Returns: - Task ID for tracking - """ - import asyncio - import uuid - - task_id = str(uuid.uuid4()) - - # Create async task for processing - async def process_async(): - try: - response = await call_next(request) - logger.debug(f"Async webhook processing completed: {task_id}") - return response - except Exception as e: - logger.error( - f"Async webhook processing failed: {task_id}, {e}", exc_info=True - ) - raise - - # Queue the task (in production, use proper task queue like Celery) - asyncio.create_task(process_async()) - - logger.debug(f"Webhook queued for async processing: {task_id}") - return task_id - def _extract_route_parameter(self, request: Request) -> Optional[str]: """Extract route parameter from webhook URL path. diff --git a/jvspatial/api/integrations/webhooks/models.py b/jvspatial/api/integrations/webhooks/models.py index 1141e46..52d4b3d 100644 --- a/jvspatial/api/integrations/webhooks/models.py +++ b/jvspatial/api/integrations/webhooks/models.py @@ -4,6 +4,7 @@ and response caching using the JVspatial GraphContext system. """ +import hashlib from datetime import datetime, timedelta, timezone from typing import Any, Dict, Optional, Tuple @@ -351,8 +352,11 @@ async def mark_idempotency_key_processed( Returns: Updated WebhookIdempotencyKey or None if not found """ - key_records = await WebhookIdempotencyKey.find({"idempotency_key": idempotency_key}) - key_record = key_records[0] if key_records else None + key_id = ( + f"o.{WebhookIdempotencyKey._entity_name()}." + + hashlib.sha256(idempotency_key.encode("utf-8")).hexdigest() + ) + key_record = await WebhookIdempotencyKey.get(key_id) if not key_record: return None @@ -382,29 +386,55 @@ async def cleanup_expired_webhook_data() -> Dict[str, int]: now = datetime.now(timezone.utc) # Clean up expired webhook events - expired_events = await WebhookEvent.find({"expires_at": {"$lt": now}}) + try: + expired_events = await WebhookEvent.find({"expires_at": {"$lt": now}}) + except TypeError: + # JsonDB persists datetimes as strings and its in-memory matcher + # cannot compare them with a datetime operand. + expired_events = [ + event + for event in await WebhookEvent.find({}) + if event.expires_at is not None and event.expires_at < now + ] for _event_count, event in enumerate(expired_events, 1): await event.delete() # Clean up expired idempotency keys - expired_keys = await WebhookIdempotencyKey.find({"expires_at": {"$lt": now}}) + try: + expired_keys = await WebhookIdempotencyKey.find({"expires_at": {"$lt": now}}) + except TypeError: + expired_keys = [ + key for key in await WebhookIdempotencyKey.find({}) if key.expires_at < now + ] - for _key_count, key in enumerate(expired_keys, 1): - await key.delete() + keys_cleaned = 0 + for key in expired_keys: + # A pending claim may have committed external side effects before a + # worker died. Keep it until an operator reconciles the outcome. + if key.is_processed: + await key.delete() + keys_cleaned += 1 # Clean up exhausted retry records older than 7 days week_ago = now - timedelta(days=7) - old_retries = await WebhookRetryRecord.find( - {"is_exhausted": True, "updated_at": {"$lt": week_ago}} - ) + try: + old_retries = await WebhookRetryRecord.find( + {"is_exhausted": True, "updated_at": {"$lt": week_ago}} + ) + except TypeError: + old_retries = [ + retry + for retry in await WebhookRetryRecord.find({"is_exhausted": True}) + if retry.updated_at < week_ago + ] for _retry_count, retry in enumerate(old_retries, 1): await retry.delete() return { "events_cleaned": len(expired_events), - "keys_cleaned": len(expired_keys), + "keys_cleaned": keys_cleaned, "retries_cleaned": len(old_retries), } diff --git a/jvspatial/api/integrations/webhooks/utils.py b/jvspatial/api/integrations/webhooks/utils.py index 336e014..b3f4b20 100644 --- a/jvspatial/api/integrations/webhooks/utils.py +++ b/jvspatial/api/integrations/webhooks/utils.py @@ -10,13 +10,16 @@ import hashlib import hmac import json -from datetime import datetime, timedelta +import logging +from datetime import datetime, timedelta, timezone from typing import Any, Dict, List, Optional, Tuple from fastapi import HTTPException, Request from jvspatial.env import env, normalize_optional_secret_string, parse_bool_basic +logger = logging.getLogger(__name__) + class WebhookConfig: """Configuration class for webhook processing.""" @@ -390,14 +393,59 @@ async def check_idempotency( return False, None except Exception: - # Fall back to in-memory manager if database is not available - return _idempotency_manager.is_duplicate(idempotency_key) + logger.exception("Webhook idempotency lookup failed") + raise HTTPException(status_code=503, detail="Idempotency check unavailable") + + +async def reserve_idempotency( + idempotency_key: str, request: Request, raw_body: bytes, ttl_seconds: int +) -> Tuple[bool, Optional[Dict[str, Any]]]: + """Atomically claim a key before invoking the webhook handler. + + An unfinished claim remains unavailable after a crash because the handler + may already have committed external effects. Operators must reconcile it. + """ + from .models import WebhookIdempotencyKey + + if len(idempotency_key) > 256: + raise HTTPException(status_code=400, detail="Idempotency key too long") + key_id = ( + f"o.{WebhookIdempotencyKey._entity_name()}." + + hashlib.sha256(idempotency_key.encode("utf-8")).hexdigest() + ) + request_hash = hashlib.sha256( + request.method.encode() + b"\0" + request.url.path.encode() + b"\0" + raw_body + ).hexdigest() + try: + record, created = await WebhookIdempotencyKey.create_if_absent( + id=key_id, + idempotency_key=idempotency_key, + request_hash=request_hash, + request_path=request.url.path, + http_method=request.method, + expires_at=datetime.now(timezone.utc) + timedelta(seconds=ttl_seconds), + ) + except Exception: + logger.exception("Webhook idempotency reservation failed") + raise HTTPException(status_code=503, detail="Idempotency check unavailable") + if created: + return False, None + if record.request_hash != request_hash: + raise HTTPException( + status_code=409, detail="Idempotency key reused for another request" + ) + if not record.is_processed: + raise HTTPException( + status_code=409, detail="Webhook processing outcome pending reconciliation" + ) + return True, record.cached_response async def store_idempotent_response( idempotency_key: Optional[str], response: Dict[str, Any], webhook_event_id: Optional[str] = None, + status_code: int = 200, ) -> None: """Store response for idempotency checking. @@ -409,19 +457,16 @@ async def store_idempotent_response( if not idempotency_key: return - try: - from .models import mark_idempotency_key_processed - - # Store in database - await mark_idempotency_key_processed( - idempotency_key=idempotency_key, - response_data=response, - webhook_event_id=webhook_event_id, - ) + from .models import mark_idempotency_key_processed - except Exception: - # Fall back to in-memory manager if database is not available - _idempotency_manager.store_response(idempotency_key, response) + result = await mark_idempotency_key_processed( + idempotency_key=idempotency_key, + response_data=response, + webhook_event_id=webhook_event_id, + status_code=status_code, + ) + if result is None: + raise RuntimeError("Idempotency reservation missing during response store") def get_webhook_config_from_env() -> WebhookConfig: @@ -492,17 +537,30 @@ async def validate_and_process_webhook( if config.hmac_secret: signature = extract_hmac_signature(request) if not signature: - raise HTTPException(status_code=400, detail="Missing HMAC signature") + raise HTTPException(status_code=401, detail="Missing HMAC signature") if not verify_hmac_signature(raw_body, signature, config.hmac_secret): raise HTTPException(status_code=401, detail="Invalid HMAC signature") + # Reject malformed payloads before reserving a durable key. A 400 must not + # strand a pending claim that prevents a corrected delivery. + try: + parsed_payload = parse_webhook_payload(raw_body, content_type) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + # Check idempotency (skip for api_key webhooks without idempotency key) idempotency_key = None if skip_idempotency else extract_idempotency_key(request) is_duplicate = False cached_response = None if not skip_idempotency: - is_duplicate, cached_response = await check_idempotency(idempotency_key) + is_duplicate, cached_response = ( + await reserve_idempotency( + idempotency_key, request, raw_body, config.idempotency_ttl + ) + if idempotency_key + else (False, None) + ) if is_duplicate: return { "raw_body": raw_body, @@ -511,12 +569,6 @@ async def validate_and_process_webhook( "is_duplicate": True, }, cached_response - # Parse payload - try: - parsed_payload = parse_webhook_payload(raw_body, content_type) - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - return { "raw_body": raw_body, "content_type": content_type, diff --git a/jvspatial/api/integrations/webhooks/webhook_auth.py b/jvspatial/api/integrations/webhooks/webhook_auth.py index 2f53da1..f61ce47 100644 --- a/jvspatial/api/integrations/webhooks/webhook_auth.py +++ b/jvspatial/api/integrations/webhooks/webhook_auth.py @@ -183,7 +183,13 @@ async def authenticate_webhook_api_key( cached = _API_KEY_CACHE.get(cache_key) if cached is not None: cached_expiry, cached_entity = cached - if cached_expiry > now and cached_entity is not None: + entity_expiry = getattr(cached_entity, "expires_at", None) + if ( + cached_expiry > now + and cached_entity is not None + and cached_entity.is_active + and (entity_expiry is None or entity_expiry.timestamp() > now) + ): api_key_entity = cached_entity cache_hit = True logger.debug( @@ -241,9 +247,11 @@ async def authenticate_webhook_api_key( def _endpoint_allowed(ep: str) -> bool: if ep.endswith("*"): - prefix = ep[:-1] - return request_path.startswith(prefix) - return request_path.startswith(ep) + prefix = ep[:-1].rstrip("/") + return request_path == prefix or request_path.startswith( + prefix + "/" + ) + return request_path == ep if not any( _endpoint_allowed(ep) for ep in api_key_entity.allowed_endpoints diff --git a/jvspatial/api/middleware/rate_limit.py b/jvspatial/api/middleware/rate_limit.py index 7504968..a81cb81 100644 --- a/jvspatial/api/middleware/rate_limit.py +++ b/jvspatial/api/middleware/rate_limit.py @@ -79,8 +79,21 @@ def _get_client_identifier(self, request: Request) -> str: Returns: Client identifier string """ - # Try to get authenticated user/API key ID for more accurate tracking - if hasattr(request.state, "user") and request.state.user: + path = request.url.path + auth_attack_path = isinstance(path, str) and path.endswith( + ( + "/auth/register", + "/auth/login", + "/auth/forgot-password", + "/auth/reset-password", + ) + ) + # Auth entry points use IP buckets even if another middleware set a user. + if ( + not auth_attack_path + and hasattr(request.state, "user") + and request.state.user + ): user = request.state.user # Extract user_id from various formats if hasattr(user, "id"): diff --git a/jvspatial/api/server.py b/jvspatial/api/server.py index 6559b8b..398fea9 100644 --- a/jvspatial/api/server.py +++ b/jvspatial/api/server.py @@ -212,9 +212,9 @@ async def _bootstrap_admin_startup(self) -> None: password = self.config.auth.bootstrap_admin_password if not email or not password: return - if len(password) < 6: + if len(password) < 12: self._logger.warning( - "bootstrap_admin_password must be at least 6 characters; skipping bootstrap" + "bootstrap_admin_password must be at least 12 characters; skipping bootstrap" ) return try: diff --git a/jvspatial/api/server_configurator.py b/jvspatial/api/server_configurator.py index 438b82d..ae2cd0b 100644 --- a/jvspatial/api/server_configurator.py +++ b/jvspatial/api/server_configurator.py @@ -92,7 +92,12 @@ async def dispatch(self, request: Request, call_next): def _configure_rate_limit_middleware(self, app: FastAPI) -> None: """Configure rate limiting middleware if rate limiting is enabled.""" server = self._server - if not server.config.rate_limit.rate_limit_enabled: + global_enabled = server.config.rate_limit.rate_limit_enabled + auth_limits_enabled = ( + server.config.auth.enabled + and server.config.rate_limit.auth_entrypoint_rate_limit_enabled + ) + if not global_enabled and not auth_limits_enabled: return try: @@ -112,7 +117,11 @@ def _configure_rate_limit_middleware(self, app: FastAPI) -> None: app.add_middleware( RateLimitMiddleware, config=rate_limits, - default_limit=server.config.rate_limit.rate_limit_default_requests, + default_limit=( + server.config.rate_limit.rate_limit_default_requests + if global_enabled + else 0 + ), default_window=server.config.rate_limit.rate_limit_default_window, backend=backend, ) @@ -166,6 +175,20 @@ def _build_rate_limit_config(self) -> Dict[str, Any]: registry._walker_registry.items(), rate_limits, api_prefix ) + if ( + server.config.auth.enabled + and server.config.rate_limit.auth_entrypoint_rate_limit_enabled + ): + for path in ( + "/auth/register", + "/auth/login", + "/auth/forgot-password", + "/auth/reset-password", + ): + rate_limits[f"{api_prefix}{path}"] = RateLimitConfig( + requests=5, window=60 + ) + for path, override in server.config.rate_limit.rate_limit_overrides.items(): full_path = ( f"{api_prefix}{path}" if not path.startswith(api_prefix) else path diff --git a/jvspatial/core/README.md b/jvspatial/core/README.md index b7818bf..d72665a 100644 --- a/jvspatial/core/README.md +++ b/jvspatial/core/README.md @@ -46,6 +46,7 @@ core/ - **`__entity_name__` is per-subclass.** Resolution: `cls.__dict__.get("__entity_name__") or cls.__name__`. Not inherited. (`entities/object.py:35-44`) - **`id` is protected.** Set in `__init__`, cannot be reassigned. (`entities/object.py:46-48`) +- **Entity attributes are declared.** `Object.__setattr__` rejects new names, including undeclared underscore names. Use Pydantic `PrivateAttr` for runtime-only state; declared private helpers may be replaced on an instance. (`entities/object.py:147-186`) - **Walker protection is on by default.** `max_steps=10000`, `max_visits_per_node=100`, `max_execution_time=300s`, `max_queue_size=1000`. Disabling globally is forbidden. (`entities/walker.py:106-115`) - **Subclass lookup honors entity-name override and caches positive hits only.** Negative caching would break later imports. (`utils.py:58-89`) - **Root is a singleton with fixed ID `n.Root.root`.** Created under async lock. (`entities/root.py`) diff --git a/jvspatial/core/context.py b/jvspatial/core/context.py index 2d6cbeb..a4c21d6 100644 --- a/jvspatial/core/context.py +++ b/jvspatial/core/context.py @@ -18,6 +18,7 @@ Union, cast, ) +from weakref import WeakKeyDictionary from jvspatial.db.database import Database from jvspatial.db.factory import create_database, get_current_database @@ -50,8 +51,9 @@ def end_request_identity_map( logger = logging.getLogger(__name__) -# Global registry to track which collections have had indexes ensured -_ensured_indexes: Set[str] = set() +# Cache index setup per database instance. A process may host multiple stores or +# schemas; a class ensured on one must not suppress indexes on another. +_ensured_indexes: WeakKeyDictionary[Any, Set[str]] = WeakKeyDictionary() # Simple performance monitor for tracking operations @@ -374,6 +376,8 @@ def __init__( PerformanceMonitor() if enable_performance_monitoring else None ) self.auto_persist_migrations = auto_persist_migrations + # Populated only by graph_transaction to invalidate parent cache entries. + self._transaction_cache_keys: Optional[Set[str]] = None # Initialize cache backend if cache_backend is None: @@ -554,6 +558,8 @@ async def _get_from_cache(self, entity_id: str) -> Optional[Any]: async def _add_to_cache(self, entity_id: str, entity: Any) -> None: """Add entity to cache.""" + if self._transaction_cache_keys is not None: + self._transaction_cache_keys.add(entity_id) imap = _request_identity_map.get() if imap is not None: imap[entity_id] = entity @@ -561,6 +567,8 @@ async def _add_to_cache(self, entity_id: str, entity: Any) -> None: async def _evict_from_cache(self, entity_id: str) -> None: """Remove an entity from request and process caches.""" + if self._transaction_cache_keys is not None: + self._transaction_cache_keys.add(entity_id) imap = _request_identity_map.get() if imap is not None: imap.pop(entity_id, None) @@ -875,7 +883,7 @@ async def delete(self, entity, cascade: bool = False) -> None: collection = self._get_collection_name(entity.type_code) db = self.database await db.delete(collection, entity.id) - await self._cache.delete(entity.id) + await self._evict_from_cache(entity.id) async def find( self, entity_class, query: Dict[str, Any], limit: Optional[int] = None @@ -1127,6 +1135,20 @@ async def atomic_increment(self, node_id: str, field: str, amount: int = 1) -> b Returns True on success, False on failure. """ + from .annotations import is_protected + from .entities.node import Node + + if not field.isidentifier() or field.startswith("_"): + raise ValueError("Invalid increment field") + node = await self.get(Node, node_id) + if node is None or not hasattr(node, field): + return False + if is_protected(type(node), field): + raise ValueError("Cannot increment a protected field") + current = getattr(node, field) + if not isinstance(current, (int, float)) or isinstance(current, bool): + raise TypeError("Increment field must be numeric") + db = self.database if self._is_mongodb(db): try: @@ -1138,8 +1160,7 @@ async def atomic_increment(self, node_id: str, field: str, amount: int = 1) -> b if result is not None: cached = await self._get_from_cache(node_id) if cached and hasattr(cached, field): - current = getattr(cached, field, 0) or 0 - object.__setattr__(cached, field, current + amount) + setattr(cached, field, current + amount) return True except Exception: logger.warning( @@ -1150,14 +1171,9 @@ async def atomic_increment(self, node_id: str, field: str, amount: int = 1) -> b ) # Fallback: read-modify-write - from .entities.node import Node - - node = await self.get(Node, node_id) - if node and hasattr(node, field): - current = getattr(node, field, 0) or 0 - setattr(node, field, current + amount) - await self.save(node) - return node is not None + setattr(node, field, current + amount) + await self.save(node) + return True # Advanced query operations for performance optimization async def find_nodes( @@ -1435,18 +1451,19 @@ async def ensure_indexes(self, entity_class: Type[T]) -> None: # Check if we've already ensured indexes for this collection collection_key = f"{collection}:{entity_class._entity_name()}" - if collection_key in _ensured_indexes: + ensured = _ensured_indexes.setdefault(self.database, set()) + if collection_key in ensured: return # Already ensured # Get index definitions from the class indexes = entity_class.get_indexes() if not indexes: - _ensured_indexes.add(collection_key) + ensured.add(collection_key) return # No indexes defined # Check if database supports indexing if not hasattr(self.database, "create_index"): - _ensured_indexes.add(collection_key) + ensured.add(collection_key) return # Database doesn't support indexing # Per-class (annotation-declared) indexes are scoped to the class's @@ -1510,7 +1527,7 @@ def _scoped(index_def: Dict[str, Any], extra: Dict[str, Any]) -> Dict[str, Any]: ) # Mark as ensured - _ensured_indexes.add(collection_key) + ensured.add(collection_key) async def find_edges_between( self, @@ -2246,6 +2263,9 @@ async def graph_transaction(database: Optional[Any] = None): ) txn = await db.begin_transaction() ctx = GraphContext(txn) + parent_ctx = _default_context_var.get() or _module_default_context + ctx._transaction_cache_keys = set() + identity_token = begin_request_identity_map() try: async with scoped_default_context_async(ctx): yield ctx @@ -2253,6 +2273,14 @@ async def graph_transaction(database: Optional[Any] = None): except BaseException: await db.rollback_transaction(txn) raise + finally: + end_request_identity_map(identity_token) + touched = ctx._transaction_cache_keys + ctx._transaction_cache_keys = None + for entity_id in touched or (): + await ctx._evict_from_cache(entity_id) + if parent_ctx is not None: + await parent_ctx._evict_from_cache(entity_id) @asynccontextmanager diff --git a/jvspatial/core/entities/node.py b/jvspatial/core/entities/node.py index 7845524..edd45fa 100644 --- a/jvspatial/core/entities/node.py +++ b/jvspatial/core/entities/node.py @@ -1341,7 +1341,8 @@ async def delete(self: "Node", cascade: bool = True) -> None: if edge_obj: incoming_edges.append(edge_obj) except Exception: - continue + logger.exception("Failed to inspect incoming edge during node deletion") + raise # Remove duplicates seen_edge_ids = set() @@ -1408,7 +1409,10 @@ async def delete(self: "Node", cascade: bool = True) -> None: # Only add nodes reachable via outgoing edges nodes_to_check.add(edge.target) except Exception: - continue + logger.exception( + "Failed to inspect outgoing edges during node deletion" + ) + raise # For each candidate node, check if it should be deleted for candidate_id in nodes_to_check: @@ -1504,7 +1508,10 @@ async def is_node_only_connected_to_deletion_set( return True except Exception: - return False + logger.exception( + "Failed to inspect connected node during cascade deletion" + ) + raise # Check if candidate is only connected to deletion set if await is_node_only_connected_to_deletion_set( @@ -1513,8 +1520,10 @@ async def is_node_only_connected_to_deletion_set( nodes_to_delete.add(candidate_id) changed = True except Exception: - # Continue even if check fails - continue + logger.exception( + "Failed to inspect cascade candidate during node deletion" + ) + raise # Remove self from nodes_to_delete (we'll delete it separately at the end) nodes_to_delete.discard(self.id) @@ -1525,7 +1534,8 @@ async def is_node_only_connected_to_deletion_set( await context.database.delete("edge", edge.id) await context._cache.delete(edge.id) except Exception: - continue + logger.exception("Failed to delete incoming edge during node deletion") + raise # Clean up outgoing edges from this node for edge in outgoing_edges: @@ -1533,7 +1543,8 @@ async def is_node_only_connected_to_deletion_set( await context.database.delete("edge", edge.id) await context._cache.delete(edge.id) except Exception: - continue + logger.exception("Failed to delete outgoing edge during node deletion") + raise # If cascade is enabled, delete all dependent nodes if cascade and nodes_to_delete: @@ -1545,7 +1556,10 @@ async def is_node_only_connected_to_deletion_set( if node: dependent_nodes.append(node) except Exception: - continue + logger.exception( + "Failed to load dependent node during cascade deletion" + ) + raise # Delete dependent nodes recursively # Each node will delete its own incoming edges and any further dependent nodes @@ -1554,8 +1568,10 @@ async def is_node_only_connected_to_deletion_set( # Recursively delete with cascade=True to handle nested dependencies await dependent_node.delete(cascade=True) except Exception: - # Continue even if dependent node deletion fails - continue + logger.exception( + "Failed to delete dependent node during cascade deletion" + ) + raise # Finally, delete this node itself. Direct delete rather than # ``context.delete`` — its "no edges left?" check would cost a COUNT, diff --git a/jvspatial/core/entities/node_query.py b/jvspatial/core/entities/node_query.py index cb55ff8..5123ac9 100644 --- a/jvspatial/core/entities/node_query.py +++ b/jvspatial/core/entities/node_query.py @@ -30,7 +30,7 @@ def __init__(self, nodes: List["Node"], source: Optional["Node"] = None) -> None async def filter( self: "NodeQuery", *, - node: Optional[Union[str, List[str]]] = None, + node: Optional[Union[str, Type["Node"], List[Union[str, Type["Node"]]]]] = None, edge: Optional[Union[str, Type["Edge"], List[Union[str, Type["Edge"]]]]] = None, direction: str = "both", **kwargs: Any, @@ -51,15 +51,19 @@ async def filter( filtered_nodes = self.nodes.copy() if node: - node_types = [node] if isinstance(node, str) else node + node_types = [node] if isinstance(node, (str, type)) else node + node_names = [ + kind if isinstance(kind, str) else kind._entity_name() + for kind in node_types + ] filtered_nodes = [ - n for n in filtered_nodes if n.__class__.__name__ in node_types + n for n in filtered_nodes if n.__class__._entity_name() in node_names ] if edge or direction != "both" or kwargs: edge_types = [] if edge: edge_types = [ - e.__name__ if inspect.isclass(e) else e + e._entity_name() if inspect.isclass(e) else e for e in (edge if isinstance(edge, list) else [edge]) ] valid_nodes = [] @@ -73,7 +77,9 @@ async def filter( ] if edge_types: connectors = [ - e for e in connectors if e.__class__.__name__ in edge_types + e + for e in connectors + if e.__class__._entity_name() in edge_types ] if kwargs: connectors = [ diff --git a/jvspatial/core/entities/object.py b/jvspatial/core/entities/object.py index fce7070..9a6ab2e 100644 --- a/jvspatial/core/entities/object.py +++ b/jvspatial/core/entities/object.py @@ -153,8 +153,12 @@ def __setattr__(self: "Object", name: str, value: Any) -> None: if valid_fields is None: valid_fields = self._get_class_hierarchy_fields() - # Check if this is a valid field in the class hierarchy or private attribute - if (name in valid_fields) or (name.startswith("_")): + # Private names must also be declared on the class hierarchy. Include + # methods and descriptors so a declared private helper can be replaced + # on an instance (for example by a test double), without admitting a + # new, undeclared underscore attribute. + private_fields = type(self)._declared_private_fields() + if name in valid_fields or name in private_fields: # Use normal Pydantic setattr for model fields or private attributes super().__setattr__(name, value) else: @@ -505,6 +509,31 @@ def _get_transient_attrs(self) -> set: # cost from ~16us to <5us). __hierarchy_fields__: ClassVar[frozenset] = frozenset() + @classmethod + def _declared_private_fields(cls) -> frozenset[str]: + """Cache the class declaration scan used by every attribute write.""" + cached = cls.__dict__.get("__private_fields_cache__") + if cached is None: + cached = frozenset( + key + for base in cls.__mro__ + for key in ( + set(getattr(base, "__private_attributes__", {}) or {}) + | { + field + for field in vars(base) + if field.startswith("_") and not field.startswith("__") + } + | { + field + for field in getattr(base, "__annotations__", {}) + if field.startswith("_") and not field.startswith("__") + } + ) + ) + type.__setattr__(cls, "__private_fields_cache__", cached) + return cached + @classmethod def _get_class_hierarchy_fields(cls: Type["Object"]) -> Set[str]: """Get all model fields from this class and its parent classes (not children). diff --git a/jvspatial/core/entities/root.py b/jvspatial/core/entities/root.py index 578d04a..6c72b51 100644 --- a/jvspatial/core/entities/root.py +++ b/jvspatial/core/entities/root.py @@ -1,6 +1,8 @@ """Root node class for jvspatial graph.""" import asyncio +import threading +import weakref from typing import Any, ClassVar, Optional, Type from typing_extensions import override @@ -16,7 +18,18 @@ class Root(Node): """ id: str = "n.Root.root" - _lock: ClassVar[asyncio.Lock] = asyncio.Lock() + _locks: ClassVar[weakref.WeakKeyDictionary] = weakref.WeakKeyDictionary() + _locks_guard: ClassVar[threading.Lock] = threading.Lock() + + @classmethod + def _loop_lock(cls) -> asyncio.Lock: + loop = asyncio.get_running_loop() + with cls._locks_guard: + lock = cls._locks.get(loop) + if lock is None: + lock = asyncio.Lock() + cls._locks[loop] = lock + return lock def __init__(self, **kwargs: Any) -> None: """Initialize Root with fixed ID. @@ -36,7 +49,7 @@ async def get(cls: Type["Root"], id: Optional[str] = None) -> "Root": # type: i Returns: Root instance """ - async with cls._lock: + async with cls._loop_lock(): id = "n.Root.root" from ..context import get_default_context diff --git a/jvspatial/core/entities/walker.py b/jvspatial/core/entities/walker.py index 54b9599..feaccd9 100644 --- a/jvspatial/core/entities/walker.py +++ b/jvspatial/core/entities/walker.py @@ -860,7 +860,7 @@ async def run(self: "Walker") -> List[Any]: if hasattr(current, "id"): self._protection.record_visit(current.id) - self._trail_tracker.record_step( + await self._trail_tracker.arecord_step( current.id if hasattr(current, "id") else str(current) ) @@ -900,9 +900,11 @@ async def run(self: "Walker") -> List[Any]: details=pv.details, ) from pv except Exception as e: - # Handle other (non-protection) errors gracefully. await self.report(f"Error during traversal: {e}") - break + raise WalkerExecutionError( + walker_class=type(self).__name__, + reason="traversal_failed", + ) from e return await self.get_report() diff --git a/jvspatial/core/entities/walker_components/walker_trail.py b/jvspatial/core/entities/walker_components/walker_trail.py index 376a9ee..12b264d 100644 --- a/jvspatial/core/entities/walker_components/walker_trail.py +++ b/jvspatial/core/entities/walker_components/walker_trail.py @@ -10,12 +10,15 @@ from __future__ import annotations import asyncio +import logging from collections import deque from typing import TYPE_CHECKING, Any, Deque, Dict, List, Optional if TYPE_CHECKING: # pragma: no cover - typing only from .trail_store import TrailStore +logger = logging.getLogger(__name__) + class WalkerTrail: """Tracks traversal steps and metadata. @@ -66,6 +69,7 @@ def __init__( ) self._store: Optional["TrailStore"] = store self._walker_id: Optional[str] = walker_id + self._pending_tasks: set[asyncio.Task] = set() def record_step( self, node_id: Any, edge_id: Optional[Any] = None, **metadata: Any @@ -93,8 +97,16 @@ def record_step( # from sync code. Skip the durable write; durability is a # best-effort guarantee for sync callers. return - # Fire and forget; the store handles its own errors. - loop.create_task(self._store.append(self._walker_id, step)) + task = loop.create_task(self._store.append(self._walker_id, step)) + self._pending_tasks.add(task) + task.add_done_callback(self._on_append_done) + + def _on_append_done(self, task: asyncio.Task) -> None: + self._pending_tasks.discard(task) + try: + task.result() + except Exception: + logger.exception("Walker trail persistence failed") async def arecord_step( self, node_id: Any, edge_id: Optional[Any] = None, **metadata: Any diff --git a/jvspatial/core/mixins/deferred_save.py b/jvspatial/core/mixins/deferred_save.py index 3c00e25..1ab0eee 100644 --- a/jvspatial/core/mixins/deferred_save.py +++ b/jvspatial/core/mixins/deferred_save.py @@ -154,6 +154,17 @@ class MyEntity(Node, DeferredSaveMixin): # Incorrect deferred_saves_auto_on_init: ClassVar[bool] = True max_pending_saves: ClassVar[Optional[int]] = None + def __init_subclass__(cls, **kwargs: Any) -> None: + """Reject entity bases that would mask the mixin's save method.""" + super().__init_subclass__(**kwargs) + bases = cls.__mro__ + if DeferredSaveMixin in bases and any( + base.__name__ in ("Object", "Node", "Edge") + and bases.index(base) < bases.index(DeferredSaveMixin) + for base in bases + ): + raise TypeError("DeferredSaveMixin must precede the entity base class") + _deferred_save_mode: bool _dirty: bool _pending_save_count: int diff --git a/jvspatial/core/utils.py b/jvspatial/core/utils.py index 277953b..1d3c6c5 100644 --- a/jvspatial/core/utils.py +++ b/jvspatial/core/utils.py @@ -42,7 +42,7 @@ async def generate_id_async(type_: str, class_name: str) -> str: @deprecated( replacement="jvspatial.core.utils.generate_id", - remove_in="0.1.0", + remove_in="0.2.0", name="jvspatial.core.utils.generate_id_async", ) def _emit() -> None: diff --git a/jvspatial/core/validate.py b/jvspatial/core/validate.py index 7e8dc85..fdeeb8a 100644 --- a/jvspatial/core/validate.py +++ b/jvspatial/core/validate.py @@ -1,6 +1,6 @@ """Graph-structure validators. -CLAUDE.md spells out the modeling convention every jvspatial application +AGENTS.md spells out the modeling convention every jvspatial application should follow: 1. Every ``Node`` is reachable from ``Root``. diff --git a/jvspatial/db/_postgres_translate.py b/jvspatial/db/_postgres_translate.py index 6d85bfc..a606e4c 100644 --- a/jvspatial/db/_postgres_translate.py +++ b/jvspatial/db/_postgres_translate.py @@ -195,9 +195,7 @@ def _translate_field_clause( if op == "$exists": fragments.append( - f"{extract_jsonb} IS NOT NULL" - if operand - else f"{extract_jsonb} IS NULL" + f"{extract_text} IS NOT NULL" if operand else f"{extract_text} IS NULL" ) continue diff --git a/jvspatial/db/dynamodb.py b/jvspatial/db/dynamodb.py index 44f16bb..df73834 100644 --- a/jvspatial/db/dynamodb.py +++ b/jvspatial/db/dynamodb.py @@ -836,9 +836,8 @@ async def process_batch(batch_items: List[Dict[str, Any]]) -> None: unprocessed = retry_response.get("UnprocessedItems", {}) if unprocessed: - # Log warning but don't fail - some items may be throttled - logger.warning( - f"Some items remain unprocessed after {max_retries} retries for collection '{collection}': {len(unprocessed.get(table_name, []))}" + raise DatabaseError( + f"DynamoDB batch write left {len(unprocessed.get(table_name, []))} items unprocessed" ) except ClientError as e: @@ -874,10 +873,8 @@ async def find_many( async def bulk_save(self, collection: str, records: List[Dict[str, Any]]) -> int: """Bulk write via :meth:`batch_write`. - DynamoDB's batch_write_item handles partial failures with - unprocessed-item retry. The count returned reflects the - records we *attempted* to write -- the underlying batch_write - logs a warning on items still unprocessed after retries. + Returns only after every record has been processed; unprocessed + writes after retries raise DatabaseError. """ if not records: return 0 diff --git a/jvspatial/db/jsondb.py b/jvspatial/db/jsondb.py index c055122..855c7e8 100644 --- a/jvspatial/db/jsondb.py +++ b/jvspatial/db/jsondb.py @@ -135,6 +135,7 @@ def _maybe_sweep_orphan_tmp_files(self) -> None: def _get_collection_dir(self, collection: str) -> Path: """Get the directory path for a collection.""" + self._validate_path_component(collection, "collection") if ( is_serverless_mode() and not self._warned_non_tmp_serverless @@ -153,17 +154,35 @@ def _get_collection_dir(self, collection: str) -> Path: # First-touch orphan sweep (cheap, idempotent, serverless-skipped). self._maybe_sweep_orphan_tmp_files() collection_dir = self.base_path / collection + if not collection_dir.resolve().is_relative_to(self.base_path): + raise ValueError("Collection path escapes database root") collection_dir.mkdir(parents=True, exist_ok=True) return collection_dir + @staticmethod + def _validate_path_component(value: str, label: str) -> None: + if ( + not isinstance(value, str) + or value in ("", ".", "..") + or "/" in value + or "\\" in value + or "\x00" in value + or Path(value).is_absolute() + ): + raise ValueError(f"Invalid {label}") + def _get_record_path(self, collection: str, record_id: str) -> Path: """Get the file path for a specific record. IDs use dot separators (format: "type.ClassName.id") which are filesystem-compatible on all platforms including Windows. """ + self._validate_path_component(record_id, "record id") collection_dir = self._get_collection_dir(collection) - return collection_dir / f"{record_id}.json" + record_path = collection_dir / f"{record_id}.json" + if not record_path.resolve().is_relative_to(self.base_path): + raise ValueError("Record path escapes database root") + return record_path @staticmethod def _list_collection_json_files(collection_dir: Path) -> List[Path]: diff --git a/jvspatial/db/postgres.py b/jvspatial/db/postgres.py index 7c2eae3..bf9540f 100644 --- a/jvspatial/db/postgres.py +++ b/jvspatial/db/postgres.py @@ -1450,8 +1450,8 @@ async def create_index( of the fields in order (``___fts``, scoped to ``entity`` when given), matching ``{"$text": {"$search": ..., "$fields": []}}``. - * ``where`` / ``index_partial_filter_expression`` — partial index - predicate (raw SQL, or the Mongo-style dialect). + * ``index_partial_filter_expression`` — partial index predicate in + the supported Mongo-style dialect. Raw ``where=`` SQL is rejected. * ``method`` — ``btree`` (default), ``hash``, ``gin``, ``gist``, ``brin``. An existing index of the same name defined by the pre-0.0.18 rules @@ -1496,12 +1496,10 @@ async def create_index( ) where_parts: List[str] = [] - partial = kwargs.get("where") - if partial: - # We don't try to parse the partial expression — caller is - # responsible for getting it right. We do require it to be a - # simple Postgres predicate string. - where_parts.append(str(partial)) + if kwargs.get("where"): + raise ValueError( + "Raw index predicates are unsupported; use a partial filter expression" + ) else: # Translate Mongo-style ``index_partial_filter_expression`` # (the cross-backend kwarg used by ``attribute(index_unique=..., @@ -1530,8 +1528,7 @@ async def create_index( raise ValueError( "PostgresDB.create_index: cannot translate " f"index_partial_filter_expression {pfe!r} to a " - "PG WHERE clause. Pass an explicit ``where=`` " - "argument or use a supported filter shape " + "PG WHERE clause. Use a supported filter shape " "(equality / $gt / $exists on safe field paths " "with scalar values)." ) diff --git a/jvspatial/db/query.py b/jvspatial/db/query.py index 76b1f82..54ba009 100644 --- a/jvspatial/db/query.py +++ b/jvspatial/db/query.py @@ -10,6 +10,8 @@ from collections import OrderedDict from typing import Any, Callable, Dict, Iterator, List, Optional, Union +import regex + from jvspatial.exceptions import QueryError # Default upper bound for the per-instance ``optimize_query`` cache. The @@ -508,10 +510,19 @@ def _match_value(value: Any, condition: Any) -> bool: flags |= re.IGNORECASE elif isinstance(condition, dict) and condition.get("$options") == "i": flags |= re.IGNORECASE + if ( + not isinstance(pattern, str) + or len(pattern) > 256 + or len(value) > 8192 + ): + return False try: - if re.search(pattern, value, flags) is None: + # Length limits alone do not stop catastrophic backtracking. + # The third-party regex engine enforces a wall-clock budget + # for each candidate evaluated by in-memory backends. + if regex.search(pattern, value, flags, timeout=0.005) is None: return False - except re.error: + except (regex.error, TimeoutError): return False elif op == "$options": pass # MongoDB-style; handled with $regex above diff --git a/jvspatial/db/sqlite.py b/jvspatial/db/sqlite.py index d298588..3ac5d4f 100644 --- a/jvspatial/db/sqlite.py +++ b/jvspatial/db/sqlite.py @@ -12,6 +12,7 @@ import contextlib import json import logging +import re import uuid from pathlib import Path from typing import ( @@ -158,11 +159,15 @@ async def _get_connection(self) -> "Connection": and self.db_path_str != ":memory:" ): logger.debug( - "SQLiteDB rebinding to a new event loop; abandoning " + "SQLiteDB rebinding to a new event loop; closing " "connection owned by %r and reconnecting on %r", self._owning_loop, current_loop, ) + # The aiosqlite worker thread outlives its owning loop. Dropping + # the reference without closing it leaks that thread and can keep + # the interpreter alive after the test or server has stopped. + await self._connection.close() self._connection = None self._owning_loop = None self._initialized = False @@ -286,16 +291,17 @@ async def _repair_non_partial_unique_indexes( def _resolve_index_where(**kwargs: Any) -> Optional[str]: """Resolve a CREATE INDEX WHERE clause from kwargs. - Honors an explicit ``where=`` string, otherwise translates a - Mongo-style partial filter under any of the names + Rejects raw ``where=`` SQL and translates a Mongo-style partial + filter under any of the names ``get_indexes()`` / annotations may emit. Returns ``None`` when no partial filter was requested. Raises ``ValueError`` when a partial filter is present but cannot be translated — never silently demote a partial unique index to a global unique. """ - explicit = kwargs.get("where") - if isinstance(explicit, str) and explicit.strip(): - return explicit.strip() + if kwargs.get("where"): + raise ValueError( + "Raw index predicates are unsupported; use a partial filter expression" + ) pfe = ( kwargs.get("index_partial_filter_expression") @@ -309,8 +315,7 @@ def _resolve_index_where(**kwargs: Any) -> Optional[str]: raise ValueError( "SQLiteDB.create_index: cannot translate " f"index_partial_filter_expression {pfe!r} to a " - "SQLite WHERE clause. Pass an explicit ``where=`` " - "argument or use a supported filter shape " + "SQLite WHERE clause. Use a supported filter shape " "(equality / $gt / $exists on safe field paths " "with scalar values)." ) @@ -354,9 +359,9 @@ async def create_index( field_or_fields: Single field name (str) or list of (field_name, direction) tuples for compound indexes unique: Whether the index should enforce uniqueness **kwargs: Partial-filter options (``partialFilterExpression``, - ``partial_filter_expression``, - ``index_partial_filter_expression``, or explicit ``where=``). - Same Mongo dialect as PostgresDB.create_index. + ``partial_filter_expression``, or + ``index_partial_filter_expression``). Raw ``where=`` SQL is + rejected. Same Mongo dialect as PostgresDB.create_index. Note: SQLite indexes on nested JSON fields use json_extract() function. @@ -381,6 +386,13 @@ async def create_index( field_names = "_".join(field.replace(".", "_") for field, _ in fields) index_name = f"idx_{collection}_{field_names}" + safe_segment = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + if not safe_segment.fullmatch(collection) or any( + not all(safe_segment.fullmatch(seg) for seg in field.split(".")) + for field, _ in fields + ): + raise ValueError("Unsafe index collection or field path") + where_sql = self._resolve_index_where(**kwargs) # Inspect on-disk definition so a prior global unique index can @@ -446,16 +458,9 @@ async def create_index( ) except Exception as e: - # Partial unique indexes protect Conversation/Interaction - # coexistence — never swallow create failures for those. - if unique and where_sql: - raise RuntimeError( - f"Failed to create partial unique index '{index_name}' " - f"on collection '{collection}': {e}" - ) from e - logger.warning( - f"Failed to create index '{index_name}' on collection '{collection}': {e}" - ) + raise RuntimeError( + f"Failed to create index '{index_name}' on collection '{collection}'" + ) from e async def close(self) -> None: """Close the underlying SQLite connection. diff --git a/jvspatial/env_adapter.py b/jvspatial/env_adapter.py index df0f533..33e6a4e 100644 --- a/jvspatial/env_adapter.py +++ b/jvspatial/env_adapter.py @@ -327,6 +327,7 @@ def server_config_overrides_from_env() -> Dict[str, Any]: "JVSPATIAL_DEFERRED_TASK_PROVIDER", "JVSPATIAL_DEFERRED_INVOKE_DISABLED", "JVSPATIAL_DEFERRED_INVOKE_SECRET", + "JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK", "JVSPATIAL_ENABLE_DEFERRED_SAVES", "JVSPATIAL_AWS_DEFERRED_TRANSPORT", "JVSPATIAL_AWS_SQS_QUEUE_URL", diff --git a/jvspatial/storage/interfaces/local.py b/jvspatial/storage/interfaces/local.py index 6ab5c28..7d34165 100644 --- a/jvspatial/storage/interfaces/local.py +++ b/jvspatial/storage/interfaces/local.py @@ -183,6 +183,28 @@ def _sanitized_version_base(self, file_path: str) -> Path: raise PathTraversalError("Path escapes root directory", path=file_path) return base + @staticmethod + def _validate_version(version: str) -> str: + if ( + not isinstance(version, str) + or not version + or version in (".", "..") + or "/" in version + or "\\" in version + or "\x00" in version + or Path(version).is_absolute() + ): + raise PathTraversalError("Invalid version identifier", path=str(version)) + return version + + @staticmethod + def _require_contained(path: Path, parent: Path) -> Path: + if not path.resolve().is_relative_to(parent.resolve()): + raise PathTraversalError( + "Version path escapes storage root", path=str(path) + ) + return path + def _http_file_url(self, file_path: str) -> str: """Full URL for HTTP GET ``{FILES_ROOT}/{file_path}`` (FileStorageService).""" return f"{self.base_url.rstrip('/')}{APIRoutes.FILES_ROOT}/{file_path}" @@ -226,10 +248,12 @@ async def create_version( # Generate version if not provided if version is None: version = f"v{datetime.now(timezone.utc).strftime('%Y%m%d_%H%M%S')}_{uuid.uuid4().hex[:8]}" + version = self._validate_version(version) # Sanitize ``file_path`` and derive sandboxed version paths. version_base = self._sanitized_version_base(file_path) version_dir = version_base.with_name(version_base.name + ".versions") + self._require_contained(version_dir, self.root_dir) await to_thread(version_dir.mkdir, parents=True, exist_ok=True) version_metadata = { @@ -243,6 +267,9 @@ async def create_version( version_file = version_dir / f"{version}.bin" metadata_file = version_dir / f"{version}.meta.json" latest_file = version_base.with_name(version_base.name + ".latest") + self._require_contained(version_file, version_dir) + self._require_contained(metadata_file, version_dir) + self._require_contained(latest_file, self.root_dir) # 1) Write content atomically (fully durable). await to_thread(atomic_write_bytes, version_file, content) @@ -274,9 +301,12 @@ async def get_version(self, file_path: str, version: str) -> Dict[str, Any]: Returns: Dictionary with version information and content """ + version = self._validate_version(version) version_base = self._sanitized_version_base(file_path) version_dir = version_base.with_name(version_base.name + ".versions") version_file = version_dir / f"{version}.bin" + self._require_contained(version_dir, self.root_dir) + self._require_contained(version_file, version_dir) if not await to_thread(version_file.exists): return None @@ -285,6 +315,7 @@ async def get_version(self, file_path: str, version: str) -> Dict[str, Any]: # Try to get metadata metadata_file = version_dir / f"{version}.meta.json" + self._require_contained(metadata_file, version_dir) metadata = {} if await to_thread(metadata_file.exists): try: @@ -314,6 +345,7 @@ async def list_versions(self, file_path: str) -> List[Dict[str, Any]]: """ version_base = self._sanitized_version_base(file_path) versions_dir = version_base.with_name(version_base.name + ".versions") + self._require_contained(versions_dir, self.root_dir) if not await to_thread(versions_dir.exists): return [] @@ -352,14 +384,18 @@ async def delete_version(self, file_path: str, version: str) -> bool: Returns: True if version was deleted, False otherwise """ + version = self._validate_version(version) version_base = self._sanitized_version_base(file_path) versions_dir = version_base.with_name(version_base.name + ".versions") + self._require_contained(versions_dir, self.root_dir) if not await to_thread(versions_dir.exists): return False version_file = versions_dir / f"{version}.bin" metadata_file = versions_dir / f"{version}.meta.json" + self._require_contained(version_file, versions_dir) + self._require_contained(metadata_file, versions_dir) deleted = False @@ -384,6 +420,7 @@ async def get_latest_version(self, file_path: str) -> Optional[Dict[str, Any]]: """ version_base = self._sanitized_version_base(file_path) latest_file = version_base.with_name(version_base.name + ".latest") + self._require_contained(latest_file, self.root_dir) if not await to_thread(latest_file.exists): return None diff --git a/jvspatial/utils/decorators.py b/jvspatial/utils/decorators.py index 881e564..c9aa78a 100644 --- a/jvspatial/utils/decorators.py +++ b/jvspatial/utils/decorators.py @@ -148,6 +148,24 @@ def unreliable_operation(): """ def decorator(func: T) -> T: + if inspect.iscoroutinefunction(func): + + @functools.wraps(func) + async def async_wrapper(*args: Any, **kwargs: Any) -> Any: + import asyncio + + current_delay = delay + for attempt in range(max_attempts): + try: + return await func(*args, **kwargs) + except Exception: + if attempt == max_attempts - 1: + raise + await asyncio.sleep(current_delay) + current_delay *= backoff + + return async_wrapper # type: ignore[return-value] + @functools.wraps(func) def wrapper(*args: Any, **kwargs: Any) -> Any: current_delay = delay diff --git a/jvspatial/version.py b/jvspatial/version.py index a364d0e..cf005ef 100644 --- a/jvspatial/version.py +++ b/jvspatial/version.py @@ -7,6 +7,6 @@ # Version follows Semantic Versioning: https://semver.org/ # Format: MAJOR.MINOR.PATCH # - MAJOR: Breaking changes -# - MINOR: New features, backward compatible +# - MINOR: New features or breaking changes while pre-1.0 # - PATCH: Bug fixes, backward compatible -__version__ = "0.0.22" +__version__ = "0.1.0" diff --git a/pyproject.toml b/pyproject.toml index 5094089..49902e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,6 +52,7 @@ dependencies = [ "schedule>=1.2.2", # Job scheduling "typing-extensions>=4.0.0", # For @override decorator and enhanced typing "aiosqlite>=0.19.0", # SQLite database backend + "regex>=2024.11.6", # Bounded-time in-memory $regex evaluation ] [project.optional-dependencies] diff --git a/tests/api/auth/oauth/test_oauth_authorize_http.py b/tests/api/auth/oauth/test_oauth_authorize_http.py index dca0f0b..1214c4a 100644 --- a/tests/api/auth/oauth/test_oauth_authorize_http.py +++ b/tests/api/auth/oauth/test_oauth_authorize_http.py @@ -12,6 +12,7 @@ * deny redirects with ``error=access_denied`` and issues no code. """ +import asyncio import base64 import hashlib import secrets @@ -46,8 +47,11 @@ def _app(tmp): oauth_enabled=True, oauth_issuer_url=ISSUER, oauth_supported_scopes=["mcp", "admin"], + bootstrap_admin_email="admin@example.com", + bootstrap_admin_password="password12345", ), ) + asyncio.run(s._auth_service.bootstrap_admin("admin@example.com", "password12345")) return s.get_app() @@ -65,21 +69,15 @@ def _pkce(): def _bearer_for_mcp_user(c): """Create a non-admin user with the ``mcp`` permission; return (bearer, user_id). - The first registered user becomes admin via the bootstrap rule, so we use - that admin's bearer to mint a SECOND user with ``roles=["user"]`` and a + The explicitly bootstrapped admin's bearer mints a user with ``roles=["user"]`` and a direct ``mcp`` permission (so its effective permissions are exactly ``{"mcp"}`` — notably NOT ``admin``). We then log in as that second user to obtain its session bearer. """ - admin_email = f"admin_{uuid.uuid4().hex}@example.com" - r = c.post( - "/api/auth/register", - json={"email": admin_email, "password": "password123"}, - ) - assert r.status_code == 200, r.text + admin_email = "admin@example.com" admin_login = c.post( "/api/auth/login", - json={"email": admin_email, "password": "password123"}, + json={"email": admin_email, "password": "password12345"}, ) assert admin_login.status_code == 200, admin_login.text admin_bearer = admin_login.json()["access_token"] @@ -90,7 +88,7 @@ def _bearer_for_mcp_user(c): headers={"Authorization": f"Bearer {admin_bearer}"}, json={ "email": user_email, - "password": "password123", + "password": "password12345", "roles": ["user"], "permissions": ["mcp"], }, @@ -104,7 +102,7 @@ def _bearer_for_mcp_user(c): user_login = c.post( "/api/auth/login", - json={"email": user_email, "password": "password123"}, + json={"email": user_email, "password": "password12345"}, ) assert user_login.status_code == 200, user_login.text return user_login.json()["access_token"], user_id diff --git a/tests/api/auth/oauth/test_oauth_authorize_login_redirect.py b/tests/api/auth/oauth/test_oauth_authorize_login_redirect.py index 461c7c3..8358321 100644 --- a/tests/api/auth/oauth/test_oauth_authorize_login_redirect.py +++ b/tests/api/auth/oauth/test_oauth_authorize_login_redirect.py @@ -82,9 +82,11 @@ def _register_public_client(c, scope="mcp admin"): def _bearer_for_user(c): """Register a user (becomes admin via bootstrap) and return its session bearer.""" email = f"u_{uuid.uuid4().hex}@example.com" - r = c.post("/api/auth/register", json={"email": email, "password": "password123"}) + r = c.post("/api/auth/register", json={"email": email, "password": "password12345"}) assert r.status_code == 200, r.text - login = c.post("/api/auth/login", json={"email": email, "password": "password123"}) + login = c.post( + "/api/auth/login", json={"email": email, "password": "password12345"} + ) assert login.status_code == 200, login.text return login.json()["access_token"] diff --git a/tests/api/auth/oauth/test_oauth_bearer_middleware.py b/tests/api/auth/oauth/test_oauth_bearer_middleware.py index f7c5157..26343c8 100644 --- a/tests/api/auth/oauth/test_oauth_bearer_middleware.py +++ b/tests/api/auth/oauth/test_oauth_bearer_middleware.py @@ -106,11 +106,11 @@ async def test_oauth_bearer_authorizes_protected_route(): email = f"s{uuid.uuid4().hex[:20]}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) login = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert login.status_code == 200, login.text session_token = login.json()["access_token"] diff --git a/tests/api/auth/oauth/test_oauth_keys.py b/tests/api/auth/oauth/test_oauth_keys.py index 6a0ecba..bc56e30 100644 --- a/tests/api/auth/oauth/test_oauth_keys.py +++ b/tests/api/auth/oauth/test_oauth_keys.py @@ -6,8 +6,10 @@ import jwt import pytest +from cryptography.fernet import Fernet from jvspatial.api.auth.oauth import keys as keystore +from jvspatial.api.auth.oauth.models import OAuthSigningKey from jvspatial.core.context import GraphContext, set_default_context from jvspatial.db.factory import create_database @@ -62,3 +64,49 @@ async def test_sign_and_verify_roundtrip(temp_context): audience="https://r.example/api/mcp", ) assert decoded["sub"] == "u_1" + + +@pytest.mark.asyncio +async def test_signing_key_encrypted_at_rest(temp_context, monkeypatch): + monkeypatch.setenv( + "JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY", Fernet.generate_key().decode() + ) + key = await keystore.ensure_signing_key() + stored = (await OAuthSigningKey.find({}))[0] + assert stored.private_pem.startswith("fernet:v1:") + assert "BEGIN PRIVATE KEY" not in stored.private_pem + assert "BEGIN PRIVATE KEY" in key.private_pem + assert (await keystore.ensure_signing_key()).private_pem == key.private_pem + await key.save() + assert (await OAuthSigningKey.find({}))[0].private_pem.startswith("fernet:v1:") + assert "private_pem" not in str(await keystore.build_jwks()) + + +@pytest.mark.asyncio +async def test_existing_key_migrates_and_wrong_key_fails_closed( + temp_context, monkeypatch +): + monkeypatch.delenv("JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY", raising=False) + original = await keystore.ensure_signing_key() + monkeypatch.setenv( + "JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY", Fernet.generate_key().decode() + ) + migrated = await keystore.ensure_signing_key() + assert migrated.private_pem == original.private_pem + assert (await OAuthSigningKey.find({}))[0].private_pem.startswith("fernet:v1:") + monkeypatch.setenv( + "JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY", Fernet.generate_key().decode() + ) + with pytest.raises(RuntimeError, match="decryption failed"): + await keystore.ensure_signing_key() + monkeypatch.delenv("JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY") + with pytest.raises(RuntimeError, match="required"): + await keystore.ensure_signing_key() + + +@pytest.mark.asyncio +async def test_production_refuses_plaintext_key_storage(temp_context, monkeypatch): + monkeypatch.delenv("JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEY", raising=False) + monkeypatch.setenv("JVSPATIAL_ENVIRONMENT", "production") + with pytest.raises(RuntimeError, match="required for production"): + await keystore.ensure_signing_key() diff --git a/tests/api/components/test_auth_middleware.py b/tests/api/components/test_auth_middleware.py index 38ecbb7..8455ecb 100644 --- a/tests/api/components/test_auth_middleware.py +++ b/tests/api/components/test_auth_middleware.py @@ -4,11 +4,14 @@ ensuring that auth settings are properly respected for all registered endpoints. """ +import asyncio import os import tempfile +from unittest.mock import patch import pytest -from fastapi import FastAPI, Request +from fastapi import Depends, FastAPI, Request +from fastapi.security import HTTPBearer from fastapi.testclient import TestClient from starlette.responses import JSONResponse @@ -16,7 +19,8 @@ from jvspatial.api.config import ServerConfig from jvspatial.api.decorators.route import endpoint from jvspatial.api.server import Server -from jvspatial.core.entities import Walker +from jvspatial.core.context import scoped_default_context +from jvspatial.core.entities import Root, Walker class TestAuthenticationMiddleware: @@ -70,6 +74,32 @@ async def protected_endpoint(): assert response.status_code == 401 assert "authentication_required" in response.json()["error_code"] + def test_dependency_name_does_not_bypass_framework_auth(self, server): + server.app = server._create_app_instance() + + async def auth_metadata(): + return "public metadata" + + @server.app.get("/api/raw/dependency", dependencies=[Depends(auth_metadata)]) + async def raw_dependency(): + return {"ok": True} + + response = TestClient(server.app).get("/api/raw/dependency") + assert response.status_code == 401 + assert response.json()["error_code"] == "authentication_required" + + def test_enforcing_fastapi_security_handles_raw_route(self, server): + server.app = server._create_app_instance() + security = HTTPBearer() + + @server.app.get("/api/raw/security", dependencies=[Depends(security)]) + async def raw_security(): + return {"ok": True} + + response = TestClient(server.app).get("/api/raw/security") + assert response.status_code in (401, 403) + assert response.json().get("error_code") != "authentication_required" + def test_registered_endpoint_with_auth_false_allows_access(self, server): """Test that registered endpoint with auth=False allows access without auth.""" @@ -129,13 +159,21 @@ class PublicWalker(Walker): server.app = server._create_app_instance() from fastapi.testclient import TestClient - client = TestClient(server.app) - # Trigger lifespan to ensure root node exists (required for walkers) - client.get("/health") - - # Try to access without auth - should succeed - response = client.post("/api/test/walker-public", json={}) - assert response.status_code == 200 + # Entering TestClient runs lifespan and creates the Root walker needs. + context = server.get_graph_context() + assert context is not None + with scoped_default_context(context): + asyncio.run(Root.get()) + with ( + scoped_default_context(context), + patch( + "jvspatial.api.endpoints.walker_executor.get_default_context", + return_value=context, + ), + TestClient(server.app) as client, + ): + response = client.post("/api/test/walker-public", json={}) + assert response.status_code == 200 def test_walker_endpoint_with_auth_true_requires_auth(self, server): """Test that walker endpoint with auth=True requires authentication.""" @@ -880,13 +918,13 @@ async def protected_endpoint(): email = f"test_{test_id}@example.com" register_response = client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert register_response.status_code == 200, register_response.text login_response = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert login_response.status_code == 200, login_response.text access_token = login_response.json()["access_token"] diff --git a/tests/api/endpoints/test_walker_executor.py b/tests/api/endpoints/test_walker_executor.py index c6d2e36..034fcf0 100644 --- a/tests/api/endpoints/test_walker_executor.py +++ b/tests/api/endpoints/test_walker_executor.py @@ -157,13 +157,18 @@ async def test_execute_traversal_walker_start_node_not_found(self, executor): executor.router.raise_error.assert_called_once() @pytest.mark.asyncio - async def test_execute_with_response_schema(self, executor): + async def test_execute_with_response_schema(self, executor, monkeypatch): """Test execution when response schema is defined.""" walker = MockDirectExecutionWalker() walker_cls = MockDirectExecutionWalker # Mock response schema - walker_cls._jvspatial_endpoint_config = {"response": MagicMock()} + monkeypatch.setattr( + walker_cls, + "_jvspatial_endpoint_config", + {"response": MagicMock()}, + raising=False, + ) result = await executor.execute_walker(walker, walker_cls) diff --git a/tests/api/test_api_keys.py b/tests/api/test_api_keys.py index ada77b2..23090ff 100644 --- a/tests/api/test_api_keys.py +++ b/tests/api/test_api_keys.py @@ -239,13 +239,13 @@ def test_create_api_key_endpoint(self, server, client, unique_email): # First, register and login a user register_response = client.post( "/api/auth/register", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) assert register_response.status_code == 200 login_response = client.post( "/api/auth/login", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) assert login_response.status_code == 200 token = login_response.json()["access_token"] @@ -270,11 +270,11 @@ def test_list_api_keys_endpoint(self, server, client, unique_email): # Register, login, and create a key client.post( "/api/auth/register", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) login_response = client.post( "/api/auth/login", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) token = login_response.json()["access_token"] @@ -302,11 +302,11 @@ def test_revoke_api_key_endpoint(self, server, client, unique_email): # Register, login, and create a key client.post( "/api/auth/register", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) login_response = client.post( "/api/auth/login", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) token = login_response.json()["access_token"] @@ -344,11 +344,11 @@ async def protected_endpoint(): # Register, login, and create a key client.post( "/api/auth/register", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) login_response = client.post( "/api/auth/login", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) token = login_response.json()["access_token"] @@ -418,7 +418,7 @@ async def add_route_protected_endpoint(): # Register, login, and create a key register_response = client.post( "/api/auth/register", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) assert ( register_response.status_code == 200 @@ -426,7 +426,7 @@ async def add_route_protected_endpoint(): login_response = client.post( "/api/auth/login", - json={"email": unique_email, "password": "password123"}, + json={"email": unique_email, "password": "password12345"}, ) assert ( login_response.status_code == 200 diff --git a/tests/api/test_auth_password_change.py b/tests/api/test_auth_password_change.py index 034e0f7..b993105 100644 --- a/tests/api/test_auth_password_change.py +++ b/tests/api/test_auth_password_change.py @@ -35,11 +35,11 @@ async def test_change_password_success(self): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) login_response = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert login_response.status_code == 200 access_token = login_response.json()["access_token"] @@ -48,8 +48,8 @@ async def test_change_password_success(self): "/api/auth/change-password", headers={"Authorization": f"Bearer {access_token}"}, json={ - "current_password": "password123", - "new_password": "newpass456", + "current_password": "password12345", + "new_password": "newpass456789", }, ) assert change_response.status_code == 200 @@ -58,14 +58,14 @@ async def test_change_password_success(self): # Old password should fail old_login = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert old_login.status_code == 401 # New password should work new_login = client.post( "/api/auth/login", - json={"email": email, "password": "newpass456"}, + json={"email": email, "password": "newpass456789"}, ) assert new_login.status_code == 200 @@ -84,11 +84,11 @@ async def test_change_password_wrong_current(self): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) login_response = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) access_token = login_response.json()["access_token"] @@ -97,7 +97,7 @@ async def test_change_password_wrong_current(self): headers={"Authorization": f"Bearer {access_token}"}, json={ "current_password": "wrongpassword", - "new_password": "newpass456", + "new_password": "newpass456789", }, ) assert change_response.status_code == 400 @@ -120,8 +120,8 @@ async def test_change_password_unauthenticated(self): change_response = client.post( "/api/auth/change-password", json={ - "current_password": "password123", - "new_password": "newpass456", + "current_password": "password12345", + "new_password": "newpass456789", }, ) assert change_response.status_code == 401 @@ -145,11 +145,11 @@ async def test_change_password_disabled_returns_404(self): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) login_response = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) access_token = login_response.json()["access_token"] @@ -157,8 +157,8 @@ async def test_change_password_disabled_returns_404(self): "/api/auth/change-password", headers={"Authorization": f"Bearer {access_token}"}, json={ - "current_password": "password123", - "new_password": "newpass456", + "current_password": "password12345", + "new_password": "newpass456789", }, ) assert change_response.status_code == 404 diff --git a/tests/api/test_auth_password_reset.py b/tests/api/test_auth_password_reset.py index 8bc906a..c036b48 100644 --- a/tests/api/test_auth_password_reset.py +++ b/tests/api/test_auth_password_reset.py @@ -44,7 +44,7 @@ async def test_forgot_password_same_response_existing_and_nonexisting(self): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) # Existing email - same message (no enumeration) @@ -82,7 +82,7 @@ async def on_reset(email, token, reset_url): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) client.post( @@ -118,7 +118,7 @@ def on_reset(email, token, reset_url): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) # Deactivate user via auth service @@ -177,7 +177,7 @@ async def test_reset_password_success(self): email = f"test_{test_id}@example.com" client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) # Request reset to get token via callback @@ -204,7 +204,7 @@ def capture_token(em, token, url): # Reset password reset_response = client2.post( "/api/auth/reset-password", - json={"token": token, "new_password": "newpass789"}, + json={"token": token, "new_password": "newpass789123"}, ) assert reset_response.status_code == 200 assert reset_response.json()["message"] == "Password reset successfully" @@ -212,14 +212,14 @@ def capture_token(em, token, url): # New password works login_response = client2.post( "/api/auth/login", - json={"email": email, "password": "newpass789"}, + json={"email": email, "password": "newpass789123"}, ) assert login_response.status_code == 200 # Old password fails old_login = client2.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert old_login.status_code == 401 @@ -237,7 +237,7 @@ async def test_reset_password_invalid_token(self): r = client.post( "/api/auth/reset-password", - json={"token": "invalid-token-xyz", "new_password": "newpass789"}, + json={"token": "invalid-token-xyz", "new_password": "newpass789123"}, ) assert r.status_code == 400 body = r.json() @@ -262,6 +262,6 @@ async def test_reset_password_disabled_returns_404(self): r = client.post( "/api/auth/reset-password", - json={"token": "any-token", "new_password": "newpass789"}, + json={"token": "any-token", "new_password": "newpass789123"}, ) assert r.status_code == 404 diff --git a/tests/api/test_auth_refresh.py b/tests/api/test_auth_refresh.py index 3708432..b637d44 100644 --- a/tests/api/test_auth_refresh.py +++ b/tests/api/test_auth_refresh.py @@ -99,7 +99,7 @@ async def test_login_generates_refresh_token(self, auth_service): mock_user.name = "Test User" mock_user.is_active = True mock_user.created_at = datetime.now(timezone.utc) - mock_user.password_hash = auth_service._hash_password("password123") + mock_user.password_hash = auth_service._hash_password("password12345") mock_user.save = AsyncMock() # Mock context methods @@ -108,7 +108,7 @@ async def test_login_generates_refresh_token(self, auth_service): auth_service._find_user_by_email = AsyncMock(return_value=mock_user) auth_service.context.save = AsyncMock() - login_data = UserLogin(email="test@example.com", password="password123") + login_data = UserLogin(email="test@example.com", password="password12345") token_response = await auth_service.login_user(login_data) assert token_response.refresh_token is not None @@ -399,13 +399,13 @@ async def test_refresh_endpoint_success(self): email = f"test_{test_id}@example.com" register_response = client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert register_response.status_code == 200, register_response.text login_response = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert login_response.status_code == 200 login_data = login_response.json() @@ -466,7 +466,7 @@ async def test_revoke_all_endpoint(self): email = f"test_{test_id}@example.com" register_response = client.post( "/api/auth/register", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert ( register_response.status_code == 200 @@ -474,7 +474,7 @@ async def test_revoke_all_endpoint(self): login_response = client.post( "/api/auth/login", - json={"email": email, "password": "password123"}, + json={"email": email, "password": "password12345"}, ) assert login_response.status_code == 200, f"Login failed: {login_response.text}" login_data = login_response.json() @@ -508,7 +508,7 @@ async def test_login_succeeds_when_refresh_token_generation_fails( mock_user.name = "Test User" mock_user.is_active = True mock_user.created_at = datetime.now(timezone.utc) - mock_user.password_hash = auth_service._hash_password("password123") + mock_user.password_hash = auth_service._hash_password("password12345") mock_user.save = AsyncMock() mock_user._graph_context = auth_service.context @@ -530,7 +530,7 @@ def log_warning(msg, *args): auth_service._logger.warning = log_warning # Attempt login - login_data = UserLogin(email="test@example.com", password="password123") + login_data = UserLogin(email="test@example.com", password="password12345") token_response = await auth_service.login_user(login_data) # Verify login succeeded despite refresh token failure @@ -554,7 +554,7 @@ async def test_login_returns_none_refresh_token_on_failure(self, auth_service): mock_user.name = "Test User" mock_user.is_active = True mock_user.created_at = datetime.now(timezone.utc) - mock_user.password_hash = auth_service._hash_password("password123") + mock_user.password_hash = auth_service._hash_password("password12345") mock_user.save = AsyncMock() mock_user._graph_context = auth_service.context @@ -567,7 +567,7 @@ async def test_login_returns_none_refresh_token_on_failure(self, auth_service): ) # Attempt login - login_data = UserLogin(email="test@example.com", password="password123") + login_data = UserLogin(email="test@example.com", password="password12345") token_response = await auth_service.login_user(login_data) # Verify refresh_token is None @@ -586,7 +586,7 @@ async def test_login_logs_warning_on_refresh_token_failure(self, auth_service): mock_user.name = "Test User" mock_user.is_active = True mock_user.created_at = datetime.now(timezone.utc) - mock_user.password_hash = auth_service._hash_password("password123") + mock_user.password_hash = auth_service._hash_password("password12345") mock_user.save = AsyncMock() mock_user._graph_context = auth_service.context @@ -607,7 +607,7 @@ def log_warning(msg, *args): auth_service._logger.warning = log_warning # Attempt login - login_data = UserLogin(email="test@example.com", password="password123") + login_data = UserLogin(email="test@example.com", password="password12345") await auth_service.login_user(login_data) # Verify warning was logged diff --git a/tests/api/test_auth_service.py b/tests/api/test_auth_service.py index 551b7ee..652bf4c 100644 --- a/tests/api/test_auth_service.py +++ b/tests/api/test_auth_service.py @@ -285,6 +285,8 @@ async def test_validate_token_user_not_found_returns_none(self, auth_service): # Mock methods auth_service._is_token_blacklisted_by_jti = AsyncMock(return_value=False) auth_service._get_user_by_id = AsyncMock(return_value=None) + auth_service._find_user_by_email = AsyncMock(return_value=None) + auth_service.context.database.find = AsyncMock(return_value=[]) # Track logger calls (service uses warning for user not found) logger_calls = [] diff --git a/tests/api/test_components.py b/tests/api/test_components.py index e4b698f..eb8bf1b 100644 --- a/tests/api/test_components.py +++ b/tests/api/test_components.py @@ -20,8 +20,9 @@ ) from jvspatial.api.components.error_handler import APIErrorHandler from jvspatial.api.config import ServerConfig -from jvspatial.core.context import GraphContext +from jvspatial.core.context import GraphContext, scoped_default_context_async from jvspatial.core.entities import Walker +from jvspatial.db.jsondb import JsonDB class TestAppBuilder: @@ -78,7 +79,7 @@ async def _lifespan(): assert isinstance(app, FastAPI) # Note: lifespan testing would require actually starting the app - async def test_register_core_routes(self, app_builder): + async def test_register_core_routes(self, app_builder, tmp_path): """Test core route registration.""" app = app_builder.create_app() @@ -89,7 +90,9 @@ async def test_register_core_routes(self, app_builder): client = TestClient(app) # Test health endpoint - response = client.get("/health") + context = GraphContext(JsonDB(base_path=str(tmp_path / "health"))) + async with scoped_default_context_async(context): + response = client.get("/health") assert response.status_code == 200 data = response.json() assert data["status"] == "healthy" diff --git a/tests/api/test_deferred_invoke_fail_closed_audit.py b/tests/api/test_deferred_invoke_fail_closed_audit.py index 6d36d52..fb80ca0 100644 --- a/tests/api/test_deferred_invoke_fail_closed_audit.py +++ b/tests/api/test_deferred_invoke_fail_closed_audit.py @@ -31,12 +31,12 @@ def test_no_secret_set_denies_non_loopback(): assert _deferred_invoke_secret_ok(req) is False -def test_no_secret_set_allows_loopback(): +def test_no_secret_set_denies_loopback_by_default(): with patch.dict(os.environ, {}, clear=False): os.environ.pop("JVSPATIAL_DEFERRED_INVOKE_SECRET", None) for host in ("127.0.0.1", "::1", "localhost", "LOCALHOST"): req = _fake_request({}, host=host) - assert _deferred_invoke_secret_ok(req) is True, host + assert _deferred_invoke_secret_ok(req) is False, host def test_no_client_denies_when_secret_unset(): @@ -46,11 +46,14 @@ def test_no_client_denies_when_secret_unset(): assert _deferred_invoke_secret_ok(req) is False -def test_loopback_allows_even_when_secret_set_without_header(): +def test_loopback_allows_when_explicitly_configured(): """LWA self-invoke cannot attach custom headers; loopback must still work.""" with patch.dict( os.environ, - {"JVSPATIAL_DEFERRED_INVOKE_SECRET": "shh"}, # pragma: allowlist secret + { + "JVSPATIAL_DEFERRED_INVOKE_SECRET": "shh", + "JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK": "true", + }, # pragma: allowlist secret clear=False, ): req = _fake_request({}, host="127.0.0.1") diff --git a/tests/api/test_session_store.py b/tests/api/test_session_store.py index ccadf3f..2fb5795 100644 --- a/tests/api/test_session_store.py +++ b/tests/api/test_session_store.py @@ -167,15 +167,14 @@ async def test_default_ttl_used_when_unset(self) -> None: _, _, ttl = cache.sets[0] assert ttl == 300 - async def test_get_error_returns_none(self) -> None: + async def test_get_error_propagates_for_fail_closed_auth(self) -> None: class _BadCache(_FakeCache): async def get(self, key: str) -> Any: raise RuntimeError("redis unreachable") store = RedisSessionStore(_BadCache(), prefix="jvs:test:") - # Treat failure as a miss so the caller falls back to the DB - # rather than crashing. - assert await store.get("k") is None + with pytest.raises(RuntimeError, match="redis unreachable"): + await store.get("k") # ---- AuthenticationService integration ------------------------------------- diff --git a/tests/api/test_storage_serve.py b/tests/api/test_storage_serve.py index 2d2d053..946bf4d 100644 --- a/tests/api/test_storage_serve.py +++ b/tests/api/test_storage_serve.py @@ -90,7 +90,7 @@ async def test_openapi_files_delete_has_security_when_get_is_public(monkeypatch) base = Path(tempfile.mkdtemp(prefix=f"jvsp_openapi_{tid}_")) root = base / "vault" db_path = base / f"db_{tid}" - monkeypatch.delenv("JVSPATIAL_FILES_PUBLIC_READ", raising=False) + monkeypatch.setenv("JVSPATIAL_FILES_PUBLIC_READ", "true") try: server = Server( title="files-openapi-security", @@ -121,8 +121,8 @@ async def test_openapi_files_delete_has_security_when_get_is_public(monkeypatch) @pytest.mark.asyncio -async def test_get_files_open_by_default_when_auth_enabled(monkeypatch): - """Default JVSPATIAL_FILES_PUBLIC_READ: anonymous GET allowed when auth middleware is on.""" +async def test_get_files_private_by_default_when_auth_enabled(monkeypatch): + """Anonymous GET is denied when auth is enabled by default.""" tid = uuid.uuid4().hex[:8] base = Path(tempfile.mkdtemp(prefix=f"jvsp_files_open_{tid}_")) root = base / "vault" @@ -153,8 +153,7 @@ async def test_get_files_open_by_default_when_auth_enabled(monkeypatch): url = f"{APIRoutes.FILES_ROOT}/smoke/hello.txt" with TestClient(app) as client: r = client.get(url) - assert r.status_code == 200 - assert r.content == b"open-bytes" + assert r.status_code == 401 finally: shutil.rmtree(base, ignore_errors=True) @@ -241,7 +240,7 @@ async def test_delete_files_requires_auth_when_get_is_public(monkeypatch): base = Path(tempfile.mkdtemp(prefix=f"jvsp_files_del_auth_{tid}_")) root = base / "vault" db_path = base / f"db_{tid}" - monkeypatch.delenv("JVSPATIAL_FILES_PUBLIC_READ", raising=False) + monkeypatch.setenv("JVSPATIAL_FILES_PUBLIC_READ", "true") try: server = Server( title="files-delete-auth-test", diff --git a/tests/api/webhook/test_decorators.py b/tests/api/webhook/test_decorators.py index bb469d7..dae1fee 100644 --- a/tests/api/webhook/test_decorators.py +++ b/tests/api/webhook/test_decorators.py @@ -40,7 +40,7 @@ async def basic_webhook(payload: dict, endpoint): assert hasattr(basic_webhook, "_jvspatial_endpoint_config") config = basic_webhook._jvspatial_endpoint_config assert config["path"] == "/webhook/basic" - assert config["methods"] == ["GET"] + assert config["methods"] == ["POST"] assert config["auth_required"] is False assert config["permissions"] == [] assert config["roles"] == [] @@ -146,7 +146,7 @@ async def process_webhook(self, payload: dict, endpoint): assert hasattr(WebhookWalker, "_jvspatial_endpoint_config") config = WebhookWalker._jvspatial_endpoint_config assert config["path"] == "/webhook/walker" - assert config["methods"] == ["GET"] + assert config["methods"] == ["POST"] assert config["webhook"] is True # Clean up diff --git a/tests/api/webhook/test_webhook_api_key_auth.py b/tests/api/webhook/test_webhook_api_key_auth.py index 4771323..6f6f237 100644 --- a/tests/api/webhook/test_webhook_api_key_auth.py +++ b/tests/api/webhook/test_webhook_api_key_auth.py @@ -45,7 +45,7 @@ def unique_email(self, request): test_id = uuid.uuid4().hex[:8] return f"test_{test_name}_{test_id}@example.com" - def _register_and_login(self, client, email, password="password123"): + def _register_and_login(self, client, email, password="password12345"): """Helper to register and login a user.""" # Try to register, ignore if already exists register_response = client.post( @@ -199,8 +199,7 @@ async def invalid_webhook(payload: dict): json={"event": "test"}, ) - # Should return 200 with error (webhooks always return 200) - assert response.status_code == 200 + assert response.status_code == 401 data = response.json() assert "error" in data or "status" in data @@ -225,8 +224,7 @@ async def missing_webhook(payload: dict): json={"event": "test"}, ) - # Should return 200 with error (webhooks always return 200) - assert response.status_code == 200 + assert response.status_code == 401 data = response.json() assert "error" in data or "status" in data diff --git a/tests/core/test_edge_ids_derive.py b/tests/core/test_edge_ids_derive.py index 69644f0..aaeb772 100644 --- a/tests/core/test_edge_ids_derive.py +++ b/tests/core/test_edge_ids_derive.py @@ -20,7 +20,11 @@ from jvspatial.cli import _run_strip_node_edges, build_parser from jvspatial.core import context as context_module -from jvspatial.core.context import GraphContext, set_default_context +from jvspatial.core.context import ( + GraphContext, + scoped_default_context_async, + set_default_context, +) from jvspatial.core.entities import Edge, Node, Root from jvspatial.core.graph_expansion import expand_node from jvspatial.db.jsondb import JsonDB @@ -296,13 +300,19 @@ async def test_context_delete_without_cascade_removes_edges(graph): async def test_root_rehydrates(graph): _ctx, _db, _backend = graph - root = await Root.get() - app = await DerivePerson.create(name="app") - await root.connect(app, edge=DeriveKnows) - - again = await Root.get() - assert {n.id for n in await again.nodes()} == {app.id} - assert await again.connection_count() == 1 + async with scoped_default_context_async(_ctx): + root = await Root.get() + app = await DerivePerson.create(name="app") + assert root._graph_context is _ctx + assert app._graph_context is _ctx + await root.connect(app, edge=DeriveKnows) + edges = await _db.find("edge", {"source": root.id}) + assert {edge["target"] for edge in edges} == {app.id} + + again = await Root.get() + assert again._graph_context is _ctx + assert {n.id for n in await again.nodes()} == {app.id} + assert await again.connection_count() == 1 async def test_expand_node_pages_from_edge_collection(graph): diff --git a/tests/core/test_graph_transaction.py b/tests/core/test_graph_transaction.py index 37052cb..4213fba 100644 --- a/tests/core/test_graph_transaction.py +++ b/tests/core/test_graph_transaction.py @@ -2,17 +2,25 @@ from __future__ import annotations -from typing import Any, Dict, List, Optional +from copy import deepcopy +from typing import Any, Dict, List, Optional, cast from unittest.mock import AsyncMock import pytest +from jvspatial.core.annotations import attribute from jvspatial.core.context import ( + GraphContext, TransactionUnavailable, + begin_request_identity_map, + end_request_identity_map, get_default_context, graph_transaction, + scoped_default_context_async, ) from jvspatial.core.entities import Edge, Node +from jvspatial.core.entities.object import Object +from jvspatial.db.database import Database class _FakeTxn: @@ -51,6 +59,35 @@ async def rollback_transaction(self, transaction: _FakeTxn) -> None: self.rolled_back = True +class _StoreTxn: + def __init__(self, rows: Dict[str, Dict[str, Any]]) -> None: + self.rows = deepcopy(rows) + + async def save(self, collection: str, data: Dict[str, Any]) -> Dict[str, Any]: + self.rows[data["id"]] = deepcopy(data) + return data + + async def get(self, collection: str, id: str) -> Optional[Dict[str, Any]]: + return deepcopy(self.rows.get(id)) + + async def delete(self, collection: str, id: str) -> bool: + return self.rows.pop(id, None) is not None + + +class _StoreDB(_StoreTxn): + def __init__(self) -> None: + super().__init__({}) + + async def begin_transaction(self) -> _StoreTxn: + return _StoreTxn(self.rows) + + async def commit_transaction(self, transaction: _StoreTxn) -> None: + self.rows = transaction.rows + + async def rollback_transaction(self, transaction: _StoreTxn) -> None: + pass + + class Folder(Node): name: str = "" @@ -63,6 +100,18 @@ class Contains(Edge): pass +class IndexedRecord(Object): + lookup: str = attribute(default="", indexed=True) + + +class _IndexDB: + def __init__(self) -> None: + self.created = 0 + + async def create_index(self, *_args: Any, **_kwargs: Any) -> None: + self.created += 1 + + @pytest.mark.asyncio async def test_graph_transaction_writes_through_the_handle_and_commits() -> None: db = _FakeDB() @@ -91,3 +140,51 @@ async def test_graph_transaction_refuses_a_store_without_begin() -> None: with pytest.raises(TransactionUnavailable): async with graph_transaction(object()): pass + + +@pytest.mark.asyncio +async def test_graph_transaction_invalidates_parent_cache_after_commit() -> None: + db = _StoreDB() + parent = GraphContext(cast(Database, db)) + async with scoped_default_context_async(parent): + identity_token = begin_request_identity_map() + try: + folder = await Folder.create(name="before") + assert (await Folder.get(folder.id)).name == "before" + async with graph_transaction(db): + inside = await Folder.get(folder.id) + inside.name = "after" + await inside.save() + assert (await Folder.get(folder.id)).name == "after" + finally: + end_request_identity_map(identity_token) + + +@pytest.mark.asyncio +async def test_graph_transaction_rollback_does_not_leak_cached_mutation() -> None: + db = _StoreDB() + parent = GraphContext(cast(Database, db)) + async with scoped_default_context_async(parent): + identity_token = begin_request_identity_map() + try: + folder = await Folder.create(name="before") + assert (await Folder.get(folder.id)).name == "before" + with pytest.raises(RuntimeError, match="abort"): + async with graph_transaction(db): + inside = await Folder.get(folder.id) + inside.name = "rolled back" + await inside.save() + raise RuntimeError("abort") + assert (await Folder.get(folder.id)).name == "before" + finally: + end_request_identity_map(identity_token) + + +@pytest.mark.asyncio +async def test_index_setup_is_scoped_to_database_instance(monkeypatch) -> None: + monkeypatch.setenv("JVSPATIAL_AUTO_CREATE_INDEXES", "true") + first, second = _IndexDB(), _IndexDB() + for database in (first, first, second): + await GraphContext(cast(Database, database)).ensure_indexes(IndexedRecord) + assert first.created > 0 + assert second.created == first.created diff --git a/tests/core/test_pager_audit_fixes.py b/tests/core/test_pager_audit_fixes.py index 9dcff91..8470b6c 100644 --- a/tests/core/test_pager_audit_fixes.py +++ b/tests/core/test_pager_audit_fixes.py @@ -42,7 +42,7 @@ def test_object_pager_has_no_cache_attribute(): @pytest.mark.asyncio -async def test_after_id_with_order_by_rejected(): +async def test_after_id_with_order_by_rejected(context): pager = ObjectPager(PageNode, page_size=3, order_by="value") with pytest.raises(ValueError, match="after_id"): await pager.get_page(after_id="n.PageNode.abc") diff --git a/tests/db/test_observable_database.py b/tests/db/test_observable_database.py index b23103b..51dbb2b 100644 --- a/tests/db/test_observable_database.py +++ b/tests/db/test_observable_database.py @@ -97,7 +97,11 @@ async def test_slow_query_elevates_to_warning(self, jsondb, caplog): await jsondb.save("node", {"id": "x", "v": 1}) with caplog.at_level(logging.WARNING, logger="jvspatial.db.observable"): await wrapped.get("node", "x") - warns = [r for r in caplog.records if r.levelno == logging.WARNING] + warns = [ + r + for r in caplog.records + if r.levelno == logging.WARNING and r.name == "jvspatial.db.observable" + ] assert warns, "expected a WARNING-level slow query log" assert "SLOW" in warns[-1].message @@ -107,7 +111,11 @@ async def test_fast_query_stays_info(self, jsondb, caplog): await jsondb.save("node", {"id": "x", "v": 1}) with caplog.at_level(logging.INFO, logger="jvspatial.db.observable"): await wrapped.get("node", "x") - warns = [r for r in caplog.records if r.levelno == logging.WARNING] + warns = [ + r + for r in caplog.records + if r.levelno == logging.WARNING and r.name == "jvspatial.db.observable" + ] assert warns == [] diff --git a/tests/db/test_postgres_integration.py b/tests/db/test_postgres_integration.py index 9e12747..2b1928c 100644 --- a/tests/db/test_postgres_integration.py +++ b/tests/db/test_postgres_integration.py @@ -190,6 +190,20 @@ async def test_find_returns_matching(self, pg_db: "PostgresDB") -> None: ids = sorted(r["id"] for r in out) assert ids == ["n.x.0", "n.x.2", "n.x.4"] + async def test_exists_treats_json_null_as_absent(self, pg_db: "PostgresDB") -> None: + for suffix, context in ( + ("missing", {}), + ("null", {"nested": {"value": None}}), + ("set", {"nested": {"value": "present"}}), + ): + await pg_db.save( + "node", {"id": f"n.x.{suffix}", "entity": "x", "context": context} + ) + absent = await pg_db.find("node", {"context.nested.value": {"$exists": False}}) + present = await pg_db.find("node", {"context.nested.value": {"$exists": True}}) + assert {row["id"] for row in absent} == {"n.x.missing", "n.x.null"} + assert {row["id"] for row in present} == {"n.x.set"} + async def test_count_with_filter(self, pg_db: "PostgresDB") -> None: for i in range(10): await pg_db.save( diff --git a/tests/db/test_postgres_translate.py b/tests/db/test_postgres_translate.py index e597b08..a9307d1 100644 --- a/tests/db/test_postgres_translate.py +++ b/tests/db/test_postgres_translate.py @@ -89,10 +89,12 @@ def test_empty_nin_matches_everything(self) -> None: class TestExistenceAndType: def test_exists_true(self) -> None: sql, _ = translate_query({"context.x": {"$exists": True}}) + assert "#>>" in sql assert sql.endswith("IS NOT NULL") def test_exists_false(self) -> None: sql, _ = translate_query({"context.x": {"$exists": False}}) + assert "#>>" in sql assert sql.endswith("IS NULL") def test_type_string(self) -> None: diff --git a/tests/db/test_query_operator_parity_audit.py b/tests/db/test_query_operator_parity_audit.py index 3222b27..4bda34d 100644 --- a/tests/db/test_query_operator_parity_audit.py +++ b/tests/db/test_query_operator_parity_audit.py @@ -101,3 +101,10 @@ def test_optimizer_markers_do_not_break_match(): # With $hint mixed in, the match still passes on the actual condition. assert QueryEngine.match(doc, {"name": "alice", "$hint": "name_idx"}) assert QueryEngine.match(doc, {"$select": ["name"]}) + + +def test_regex_backtracking_has_a_runtime_limit(): + # A 30-byte candidate was enough to stall the standard re engine. + assert not QueryEngine.match( + {"name": "a" * 29 + "!"}, {"name": {"$regex": "^(a+)+$"}} + ) diff --git a/tests/db/test_sqlite_cross_loop_audit.py b/tests/db/test_sqlite_cross_loop_audit.py index 272b48c..d820a15 100644 --- a/tests/db/test_sqlite_cross_loop_audit.py +++ b/tests/db/test_sqlite_cross_loop_audit.py @@ -9,6 +9,7 @@ import asyncio import tempfile +from unittest.mock import patch import pytest @@ -35,6 +36,8 @@ async def first() -> None: await db.save("widgets", {"id": "w1", "qty": 1}) asyncio.run(first()) + old_connection = db._connection + assert old_connection is not None async def second() -> None: # Auto-rebind on a new loop — no error. @@ -43,7 +46,9 @@ async def second() -> None: assert got is not None await db.close() - asyncio.run(second()) + with patch.object(old_connection, "close", wraps=old_connection.close) as close: + asyncio.run(second()) + close.assert_awaited_once() def test_owning_loop_tracked(): diff --git a/tests/security/test_combined_review_regressions.py b/tests/security/test_combined_review_regressions.py new file mode 100644 index 0000000..2954452 --- /dev/null +++ b/tests/security/test_combined_review_regressions.py @@ -0,0 +1,557 @@ +"""Regression checks for the combined security and gap review.""" + +import asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest +from fastapi import FastAPI, HTTPException, Request, Response + +from jvspatial.api.auth.config import AuthConfig +from jvspatial.api.auth.models import ( + UserCreate, + UserCreateAdmin, + UserLogin, + UserRolesUpdate, +) +from jvspatial.api.auth.service import AuthenticationService +from jvspatial.api.components.app_builder import AppBuilder +from jvspatial.api.components.auth_middleware import AuthenticationMiddleware +from jvspatial.api.config import ServerConfig +from jvspatial.api.integrations.storage.service import FileStorageService +from jvspatial.api.integrations.webhooks.middleware import WebhookMiddleware +from jvspatial.api.integrations.webhooks.models import cleanup_expired_webhook_data +from jvspatial.api.integrations.webhooks.utils import ( + WebhookConfig, + generate_hmac_signature, + reserve_idempotency, + store_idempotent_response, +) +from jvspatial.api.server_configurator import ServerConfigurator +from jvspatial.core.context import GraphContext, scoped_default_context +from jvspatial.core.entities import Node +from jvspatial.core.entities.node_query import NodeQuery +from jvspatial.core.entities.root import Root +from jvspatial.core.mixins.deferred_save import DeferredSaveMixin +from jvspatial.db.jsondb import JsonDB +from jvspatial.db.sqlite import SQLiteDB +from jvspatial.exceptions import DatabaseError +from jvspatial.storage.exceptions import PathTraversalError +from jvspatial.storage.interfaces.local import LocalFileInterface +from jvspatial.utils.decorators import retry + + +def _auth(tmp_path): + return AuthenticationService( + GraphContext(database=JsonDB(str(tmp_path / "auth"))), + jwt_secret="combined-security-review-test-secret-12345", + ) + + +@pytest.mark.asyncio +async def test_user_lookup_error_cannot_authenticate_jwt(tmp_path): + service = _auth(tmp_path) + token, _ = service._generate_jwt_token( + "o.User.fake", "u@example.com", roles=["admin"] + ) + service._is_token_blacklisted_by_jti = AsyncMock(return_value=False) + service._get_user_by_id = AsyncMock(side_effect=DatabaseError("unavailable")) + assert await service.validate_token(token) is None + + +@pytest.mark.asyncio +async def test_demoted_user_loses_jwt_role(tmp_path): + service = _auth(tmp_path) + user = await service.create_user_with_roles( + UserCreateAdmin( + email="admin@example.com", password="long-password-123", roles=["admin"] + ) + ) + token, _ = service._generate_jwt_token(user.id, user.email, roles=["admin"]) + assert "admin" in (await service.validate_token(token)).roles + await service.update_user_roles(user.id, UserRolesUpdate(roles=["user"])) + # Revocation may reject the old token outright; either outcome is safe. + result = await service.validate_token(token) + assert result is None or "admin" not in result.roles + + +@pytest.mark.asyncio +async def test_concurrent_public_registration_never_grants_admin(tmp_path): + service = _auth(tmp_path) + users = await asyncio.gather( + service.register_user( + UserCreate(email="first@example.com", password="long-password-123") + ), + service.register_user( + UserCreate(email="second@example.com", password="long-password-123") + ), + ) + assert all(user.roles == ["user"] for user in users) + + +@pytest.mark.asyncio +async def test_admin_bootstrap_aborts_on_unreadable_user(tmp_path): + service = _auth(tmp_path) + service._find_user_by_email = AsyncMock(return_value=None) + service._user_count = AsyncMock(return_value=1) + service.context.database.find = AsyncMock(return_value=[{"id": "unreadable"}]) + service.context._deserialize_entity = AsyncMock(side_effect=RuntimeError("bad row")) + with pytest.raises(RuntimeError, match="bad row"): + await service.bootstrap_admin("admin@example.com", "long-password-123") + + +@pytest.mark.asyncio +async def test_logout_deactivates_bound_refresh_token(tmp_path): + service = _auth(tmp_path) + await service.register_user( + UserCreate(email="logout@example.com", password="long-password-123") + ) + tokens = await service.login_user( + UserLogin(email="logout@example.com", password="long-password-123") + ) + assert tokens.refresh_token + assert await service.validate_token(tokens.access_token) + assert await service.logout_user(tokens.access_token) + assert await service.validate_token(tokens.access_token) is None + with pytest.raises(ValueError, match="Invalid or expired refresh token"): + await service.refresh_access_token(tokens.refresh_token) + + +def test_jsondb_rejects_escaping_paths_before_creating_files(tmp_path): + db = JsonDB(str(tmp_path / "db")) + with pytest.raises(ValueError): + db._get_record_path("../outside", "id") + with pytest.raises(ValueError): + db._get_record_path("node", "../outside") + assert not (tmp_path / "outside.json").exists() + + +@pytest.mark.asyncio +async def test_edge_delete_failure_preserves_node(tmp_path, monkeypatch): + database = JsonDB(str(tmp_path / "graph")) + context = GraphContext(database=database) + parent, child = Node(), Node() + parent._graph_context = context + child._graph_context = context + await context.save(parent) + await context.save(child) + with scoped_default_context(context): + await parent.connect(child) + + original_delete = database.delete + + async def fail_edge_delete(collection, record_id): + if collection == "edge": + raise RuntimeError("edge write failed") + return await original_delete(collection, record_id) + + monkeypatch.setattr(database, "delete", fail_edge_delete) + with scoped_default_context(context): + with pytest.raises(RuntimeError, match="edge write failed"): + await child.delete(cascade=False) + assert await database.get("node", child.id) is not None + + +@pytest.mark.asyncio +async def test_cascade_discovery_failure_preserves_parent(tmp_path, monkeypatch): + database = JsonDB(str(tmp_path / "graph")) + context = GraphContext(database=database) + parent, child = Node(), Node() + parent._graph_context = context + child._graph_context = context + await context.save(parent) + await context.save(child) + with scoped_default_context(context): + await parent.connect(child) + + async def fail_edge_lookup(self, graph_context): + raise RuntimeError("edge lookup failed") + + monkeypatch.setattr(Node, "_incident_edges", fail_edge_lookup) + with scoped_default_context(context): + with pytest.raises(RuntimeError, match="edge lookup failed"): + await parent.delete() + assert await database.get("node", parent.id) is not None + + +@pytest.mark.asyncio +async def test_node_query_uses_entity_name_override(): + class NamedNode(Node): + __entity_name__ = "PublicName" + + source = Node() + named = NamedNode() + query = NodeQuery([named], source=source) + assert await query.filter(node="PublicName") == [named] + assert await query.filter(node=NamedNode) == [named] + + +@pytest.mark.asyncio +async def test_atomic_increment_rejects_missing_and_protected_fields(tmp_path): + context = GraphContext(database=JsonDB(str(tmp_path / "increment"))) + node = Node() + node._graph_context = context + await context.save(node) + assert await context.atomic_increment(node.id, "missing") is False + with pytest.raises(ValueError, match="protected"): + await context.atomic_increment(node.id, "id") + + +def test_undeclared_private_attribute_is_rejected(): + node = Node() + with pytest.raises(AttributeError): + node._unexpected = "hidden" + + +def test_declared_private_method_can_be_replaced_on_an_instance(): + class PluginNode(Node): + async def _load_token(self): + return "real" + + node = PluginNode() + replacement = AsyncMock(return_value="test") + node._load_token = replacement + assert node._load_token is replacement + with pytest.raises(AttributeError): + node._unknown_helper = replacement + + +def test_auth_rate_limit_bypass_is_explicit_and_defaults_on(): + config = ServerConfig() + config.auth.enabled = True + registry = SimpleNamespace(_function_registry={}, _walker_registry={}) + server = SimpleNamespace(config=config, _endpoint_registry=registry) + configurator = ServerConfigurator(server) + + assert config.rate_limit.auth_entrypoint_rate_limit_enabled is True + assert "/api/auth/reset-password" in configurator._build_rate_limit_config() + + config.rate_limit.auth_entrypoint_rate_limit_enabled = False + assert "/api/auth/reset-password" not in configurator._build_rate_limit_config() + + +@pytest.mark.asyncio +async def test_version_id_cannot_escape_storage_root(tmp_path): + storage = LocalFileInterface(root_dir=str(tmp_path / "files")) + with pytest.raises(PathTraversalError): + await storage.create_version("safe.txt", b"data", version="../../outside") + with pytest.raises(PathTraversalError): + await storage.get_version("safe.txt", "../../outside") + assert not (tmp_path / "outside.bin").exists() + + +@pytest.mark.asyncio +async def test_signed_get_webhook_rejects_missing_signature(): + middleware = WebhookMiddleware( + FastAPI(), config=WebhookConfig(https_required=False), server=None + ) + request = Request( + { + "type": "http", + "method": "GET", + "scheme": "http", + "path": "/webhook/check", + "raw_path": b"/webhook/check", + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 1234), + "server": ("testserver", 80), + } + ) + with pytest.raises(HTTPException) as exc: + await middleware._process_webhook_request( + request, + { + "signature_required": True, + "hmac_secret": "signed-get-test-secret", + "https_required": False, + }, + ) + assert exc.value.status_code == 401 + + +@pytest.mark.asyncio +async def test_signed_get_webhook_binds_query_string(): + secret = "signed-get-test-secret" + signature = generate_hmac_signature(b"event=approved", secret) + middleware = WebhookMiddleware( + FastAPI(), config=WebhookConfig(https_required=False), server=None + ) + config = { + "signature_required": True, + "hmac_secret": secret, + "https_required": False, + } + signed = _request("/webhook/check", headers=[(b"x-signature", signature.encode())]) + signed.scope["query_string"] = b"event=approved" + await middleware._process_webhook_request(signed, config) + assert signed.state.hmac_verified is True + + tampered = _request( + "/webhook/check", headers=[(b"x-signature", signature.encode())] + ) + tampered.scope["query_string"] = b"event=denied" + with pytest.raises(HTTPException) as exc: + await middleware._process_webhook_request(tampered, config) + assert exc.value.status_code == 401 + + +@pytest.mark.asyncio +async def test_webhook_idempotency_claim_is_durable_and_rejects_inflight(tmp_path): + context = GraphContext(database=JsonDB(str(tmp_path / "webhook"))) + request = _request("/webhook/order") + request.scope["method"] = "POST" + with scoped_default_context(context): + assert await reserve_idempotency("order-1", request, b"{}", 3600) == ( + False, + None, + ) + with pytest.raises(HTTPException) as pending: + await reserve_idempotency("order-1", request, b"{}", 3600) + assert pending.value.status_code == 409 + await store_idempotent_response("order-1", {"ok": True}) + assert await reserve_idempotency("order-1", request, b"{}", 3600) == ( + True, + {"ok": True}, + ) + with pytest.raises(HTTPException) as changed: + await reserve_idempotency("order-1", request, b'{"changed":true}', 3600) + assert changed.value.status_code == 409 + + +@pytest.mark.asyncio +async def test_webhook_middleware_replays_completed_response_once(tmp_path): + import httpx + + context = GraphContext(database=JsonDB(str(tmp_path / "webhook-middleware"))) + app = FastAPI() + app.add_middleware(WebhookMiddleware, config=WebhookConfig(https_required=False)) + calls = [] + + @app.post("/webhook/order") + async def handler(): + calls.append(1) + return {"count": len(calls)} + + transport = httpx.ASGITransport(app=app) + with scoped_default_context(context): + async with httpx.AsyncClient( + transport=transport, base_url="http://test" + ) as client: + headers = {"X-Idempotency-Key": "order-42"} + first = await client.post("/webhook/order", json={}, headers=headers) + second = await client.post("/webhook/order", json={}, headers=headers) + assert first.status_code == 200 + assert second.status_code == 200 + assert first.json() == second.json() == {"count": 1} + assert len(calls) == 1 + + +@pytest.mark.asyncio +async def test_invalid_webhook_json_does_not_claim_key(tmp_path): + import httpx + + context = GraphContext(database=JsonDB(str(tmp_path / "invalid-webhook"))) + app = FastAPI() + app.add_middleware(WebhookMiddleware, config=WebhookConfig(https_required=False)) + + @app.post("/webhook/order") + async def handler(): + return {"ok": True} + + transport = httpx.ASGITransport(app=app) + with scoped_default_context(context): + async with httpx.AsyncClient( + transport=transport, base_url="http://test" + ) as client: + headers = {"X-Idempotency-Key": "retry-after-400"} + invalid = await client.post( + "/webhook/order", + content=b"{invalid", + headers={**headers, "Content-Type": "application/json"}, + ) + valid = await client.post("/webhook/order", json={}, headers=headers) + assert invalid.status_code == 400 + assert valid.status_code == 200 + + +@pytest.mark.asyncio +async def test_concurrent_webhook_claim_has_one_winner(tmp_path): + context = GraphContext(database=JsonDB(str(tmp_path / "concurrent-webhook"))) + request = _request("/webhook/order") + request.scope["method"] = "POST" + + async def claim(): + try: + return await reserve_idempotency("shared-key", request, b"{}", 3600) + except HTTPException as exc: + return exc.status_code + + with scoped_default_context(context): + outcomes = await asyncio.gather(claim(), claim()) + assert outcomes.count((False, None)) == 1 + assert outcomes.count(409) == 1 + + +@pytest.mark.asyncio +async def test_expired_pending_webhook_claim_requires_reconciliation(tmp_path): + context = GraphContext(database=JsonDB(str(tmp_path / "pending-webhook"))) + request = _request("/webhook/order") + request.scope["method"] = "POST" + with scoped_default_context(context): + assert await reserve_idempotency("pending-key", request, b"{}", -1) == ( + False, + None, + ) + assert (await cleanup_expired_webhook_data())["keys_cleaned"] == 0 + with pytest.raises(HTTPException) as pending: + await reserve_idempotency("pending-key", request, b"{}", 3600) + assert pending.value.status_code == 409 + + +@pytest.mark.asyncio +async def test_proxy_creation_awaits_manager_and_uses_code(): + interface = SimpleNamespace( + file_exists=AsyncMock(return_value=True), base_url="https://files.example" + ) + manager = SimpleNamespace( + create_proxy=AsyncMock(return_value=SimpleNamespace(code="abc123")) + ) + service = FileStorageService(interface, proxy_manager=manager) + result = await service.handle_create_proxy("safe.txt") + manager.create_proxy.assert_awaited_once() + assert result["code"] == "abc123" + assert result["proxy_url"].endswith("/abc123") + + +def _request(path: str, *, headers=()): + return Request( + { + "type": "http", + "method": "GET", + "scheme": "http", + "path": path, + "raw_path": path.encode(), + "query_string": b"", + "headers": list(headers), + "client": ("127.0.0.1", 1234), + "server": ("testserver", 80), + } + ) + + +@pytest.mark.asyncio +async def test_endpoint_config_lookup_error_denies_request(): + middleware = AuthenticationMiddleware( + FastAPI(), AuthConfig(auth_enabled=True, test_mode=True), SimpleNamespace() + ) + request = _request("/api/private") + request.state.user = SimpleNamespace(id="user", roles=["user"], permissions=[]) + middleware.path_matcher.is_exempt = lambda _: False + middleware._auth_resolver.endpoint_requires_auth = lambda _: True + middleware._auth_resolver.endpoint_has_fastapi_auth = lambda _: False + middleware._auth_resolver.get_endpoint_config = lambda _: 1 / 0 + middleware._normalize_user = AsyncMock(return_value=request.state.user) + response = await middleware.dispatch(request, AsyncMock()) + assert response.status_code == 403 + + +@pytest.mark.asyncio +async def test_unconfigured_route_requires_login_but_no_unlisted_role(): + middleware = AuthenticationMiddleware( + FastAPI(), AuthConfig(auth_enabled=True, test_mode=True), SimpleNamespace() + ) + middleware.path_matcher.is_exempt = lambda _: False + middleware._auth_resolver.endpoint_requires_auth = lambda _: True + middleware._auth_resolver.endpoint_has_fastapi_auth = lambda _: False + middleware._auth_resolver.get_endpoint_config = lambda _: None + middleware._normalize_user = AsyncMock(side_effect=lambda user: user) + next_handler = AsyncMock(return_value=Response(status_code=200)) + + request = _request("/api/mcp") + request.state.user = SimpleNamespace(id="user", roles=["user"], permissions=[]) + response = await middleware.dispatch(request, next_handler) + assert response.status_code == 200 + + admin_request = _request("/api/graph") + admin_request.state.user = request.state.user + response = await middleware.dispatch(admin_request, next_handler) + assert response.status_code == 403 + + +@pytest.mark.asyncio +async def test_api_key_scope_uses_path_boundary(): + key = SimpleNamespace( + id="key", + user_id="user", + allowed_ips=[], + allowed_endpoints=["/api/files"], + permissions=[], + rate_limit_override=None, + ) + key_service = SimpleNamespace( + validate_key=AsyncMock(return_value=key), update_key_usage=AsyncMock() + ) + middleware = AuthenticationMiddleware( + FastAPI(), + AuthConfig(auth_enabled=True), + SimpleNamespace(_api_key_service=key_service), + ) + header = [(b"x-api-key", b"secret-for-test")] + assert ( + await middleware._authenticate_api_key( + _request("/api/files-admin", headers=header) + ) + is None + ) + assert await middleware._authenticate_api_key( + _request("/api/files", headers=header) + ) + + +@pytest.mark.asyncio +async def test_async_retry_awaits_and_retries(): + calls = [] + + @retry(max_attempts=3, delay=0) + async def sometimes_fails(): + calls.append(1) + if len(calls) < 3: + raise RuntimeError("retry") + return "ok" + + assert await sometimes_fails() == "ok" + assert len(calls) == 3 + + +def test_root_lock_is_scoped_to_event_loop(): + first = asyncio.run(_get_root_lock()) + second = asyncio.run(_get_root_lock()) + assert first is not second + + +async def _get_root_lock(): + return Root._loop_lock() + + +def test_wrong_deferred_save_mro_raises_at_class_creation(): + with pytest.raises(TypeError, match="must precede"): + + class Bad(Node, DeferredSaveMixin): + pass + + +@pytest.mark.asyncio +async def test_sqlite_index_rejects_unsafe_field(tmp_path): + db = SQLiteDB(str(tmp_path / "index.db")) + with pytest.raises(ValueError, match="Unsafe index"): + await db.create_index("node", "context.name'escape") + await db.close() + + +def test_production_docs_are_unpublished_by_default(monkeypatch): + monkeypatch.setenv("JVSPATIAL_ENVIRONMENT", "production") + monkeypatch.delenv("JVSPATIAL_DOCS_DISABLED", raising=False) + app = AppBuilder(ServerConfig()).create_app() + assert app.docs_url is None + assert app.openapi_url is None diff --git a/tests/storage/integration/test_storage_workflows.py b/tests/storage/integration/test_storage_workflows.py index 11784a6..9818462 100644 --- a/tests/storage/integration/test_storage_workflows.py +++ b/tests/storage/integration/test_storage_workflows.py @@ -754,8 +754,8 @@ async def test_large_file_upload_and_retrieval(self, temp_storage_dir): root_dir=temp_storage_dir, validator=validator ) - large_file = b"x" * (10 * 1024 * 1024) # 10 MB - file_path = "large/bigfile.dat" + large_file = b"ordinary text\n" * (10 * 1024 * 1024 // 14) + file_path = "large/bigfile.txt" # Upload result = await storage_interface.save_file(file_path, large_file) diff --git a/tests/storage/test_managers.py b/tests/storage/test_managers.py index 2f609b7..e78413a 100644 --- a/tests/storage/test_managers.py +++ b/tests/storage/test_managers.py @@ -13,7 +13,7 @@ import pytest -from jvspatial.core.context import GraphContext +from jvspatial.core.context import GraphContext, scoped_default_context_async from jvspatial.storage.exceptions import ( AccessDeniedError, FileNotFoundError, @@ -155,13 +155,14 @@ async def test_init_with_context(self, mock_context): assert manager._context == mock_context assert await manager.context == mock_context - async def test_init_without_context(self): + async def test_init_without_context(self, mock_context): """Test initialization without context uses default.""" manager = URLProxyManager() assert manager._context is None # Context will be created on first access - context = await manager.context - assert context is not None + async with scoped_default_context_async(mock_context): + context = await manager.context + assert context is mock_context async def test_get_proxy_manager_function(self, mock_context): """Test get_proxy_manager convenience function.""" diff --git a/tests/storage/test_security.py b/tests/storage/test_security.py index e02ca7e..8e36cb6 100644 --- a/tests/storage/test_security.py +++ b/tests/storage/test_security.py @@ -404,10 +404,12 @@ async def test_jpeg_detection(self): mime = validator.detect_mime_type(jpeg_content, "image.jpg") assert "image" in mime.lower() - async def test_png_detection(self): + async def test_png_detection(self, monkeypatch): """Test PNG MIME type detection.""" # PNG header png_content = b"\x89PNG\r\n\x1a\n" + # Header-only fixture is not a complete PNG for libmagic. + monkeypatch.setattr("jvspatial.storage.security.validator.HAS_MAGIC", False) validator = FileValidator() mime = validator.detect_mime_type(png_content, "image.png") assert "image" in mime.lower() @@ -541,10 +543,11 @@ async def test_allowed_image_jpeg(self): result = validator.validate_file(content, "photo.jpg") assert result["valid"] is True - async def test_allowed_image_png(self): + async def test_allowed_image_png(self, monkeypatch): """Test that PNG images are allowed.""" validator = FileValidator() content = b"\x89PNG\r\n\x1a\n" + monkeypatch.setattr("jvspatial.storage.security.validator.HAS_MAGIC", False) result = validator.validate_file(content, "graphic.png") assert result["valid"] is True @@ -666,11 +669,13 @@ async def test_none_content_raises_error(self): with pytest.raises((AttributeError, TypeError)): validator.validate_file(None, "file.txt") - async def test_binary_data_validated(self): + async def test_binary_data_validated(self, monkeypatch): """Test that binary data is properly validated.""" # Use a different extension since .bin is blocked validator = FileValidator() binary_data = bytes(range(256)) + # This test exercises the size result, not libmagic classification. + monkeypatch.setattr("jvspatial.storage.security.validator.HAS_MAGIC", False) # Use .dat which isn't explicitly blocked result = validator.validate_file(binary_data, "data.txt") assert result["size_bytes"] == 256 @@ -948,18 +953,20 @@ async def test_combined_usage_example(self): class TestHintMime: """Tests for the hint_mime parameter in detect_mime_type and validate_file.""" - def test_detect_mime_type_hint_used_when_extension_missing(self): + def test_detect_mime_type_hint_used_when_extension_missing(self, monkeypatch): """hint_mime is used when neither magic nor extension can detect the type.""" validator = FileValidator() + monkeypatch.setattr("jvspatial.storage.security.validator.HAS_MAGIC", False) content = b"\x00\x01\x02\x03unknown_binary" mime = validator.detect_mime_type( content, filename="0_upload", hint_mime="image/jpeg" ) assert mime == "image/jpeg" - def test_detect_mime_type_hint_ignored_when_extension_works(self): + def test_detect_mime_type_hint_ignored_when_extension_works(self, monkeypatch): """hint_mime is not used when extension-based detection succeeds.""" validator = FileValidator() + monkeypatch.setattr("jvspatial.storage.security.validator.HAS_MAGIC", False) content = b"plain text" mime = validator.detect_mime_type( content, filename="photo.jpg", hint_mime="image/png" @@ -982,9 +989,10 @@ def test_detect_mime_type_none_hint(self): mime = validator.detect_mime_type(content, filename="upload", hint_mime=None) assert mime == "application/octet-stream" - def test_validate_file_with_hint_mime(self): + def test_validate_file_with_hint_mime(self, monkeypatch): """validate_file accepts hint_mime and uses it when detection fails.""" validator = FileValidator() + monkeypatch.setattr("jvspatial.storage.security.validator.HAS_MAGIC", False) content = b"\x00\x01\x02\x03jpeg_binary" result = validator.validate_file( content, filename="0_upload", hint_mime="image/jpeg" diff --git a/tests/utils/test_deprecation.py b/tests/utils/test_deprecation.py index 68aa62f..b3ed65f 100644 --- a/tests/utils/test_deprecation.py +++ b/tests/utils/test_deprecation.py @@ -12,7 +12,8 @@ @pytest.fixture(autouse=True) -def reset_state(): +def reset_state(monkeypatch): + monkeypatch.setattr("jvspatial.utils.deprecation.is_serverless_mode", lambda: False) reset_deprecation_warnings() yield reset_deprecation_warnings() diff --git a/tests/utils/test_wave4_polish_audit.py b/tests/utils/test_wave4_polish_audit.py index ddc6b04..eced40d 100644 --- a/tests/utils/test_wave4_polish_audit.py +++ b/tests/utils/test_wave4_polish_audit.py @@ -29,6 +29,9 @@ async def test_generate_id_async_emits_deprecation(): (audit §3.11). It must still work — the call site only sees a warning.""" from jvspatial.core.utils import generate_id_async + from jvspatial.utils.deprecation import reset_deprecation_warnings + + reset_deprecation_warnings() with warnings.catch_warnings(record=True) as caught: warnings.simplefilter("always", DeprecationWarning)