The startup sweep was reading settings.qdrant_collection (the raw config
value, default "nextcloud_content") instead of settings.get_collection_name(),
which is what every other vector-sync operation uses. When QDRANT_COLLECTION
is not overridden, get_collection_name() auto-generates a
{deployment-id}-{model-name} name; the sweep was targeting a non-existent
collection and silently returning (0, 0).
Also adds the AsyncQdrantClient type annotation that was missing on
sweep_orphan_placeholders, and renames its parameter from collection_name
to collection to make it clear the value must be the resolved name.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the per-tenant nextcloud-mcp-server Pod OOMKills mid-batch, the
in-memory anyio processor queue is lost but the placeholder Qdrant
points (is_placeholder=true, status=pending) survive. The next Pod's
scanner re-runs, sees the existing placeholders, applies the
5 × VECTOR_SYNC_SCAN_INTERVAL staleness gate (~5h with the deployed
1h scan interval), and skips them. Result: 0 documents indexed for
the duration of the gate after every restart.
Stamps a process-level instance_id (UUID per Pod-process) onto every
placeholder write. A new sweep_orphan_placeholders helper, called
once from starlette_lifespan after the Qdrant client is initialised
and before the scanner / user-manager spawns, scrolls the collection
and deletes any placeholder whose instance_id doesn't match the
current Pod's (including placeholders with no instance_id field —
back-compat for pre-fix Pod versions). The scanner's next cycle
naturally re-creates fresh placeholders and queues work normally;
no DocumentTask reconstruction needed.
Sweep is one-shot at startup, not periodic — the existing staleness
gate still covers same-Pod recovery, and the cross-Pod-restart gap
was the only failure mode. Failure is non-fatal (logged via
vector_sync.orphan_sweep_failed) so a transient Qdrant hiccup at
boot doesn't prevent the scanner from running.
Both lifespan branches (single-user BasicAuth, OAuth / multi-user
BasicAuth) call the sweep via a module-local helper. A new
VECTOR_SYNC_ORPHAN_SWEEP_ENABLED setting (default True) provides
an escape hatch.
Closes Deck #101.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Astrolabe (and any other PHP-side client) sends a stable User-Agent on
every outbound call to the MCP server. Capture it at the middleware
layer so backend access logs can attribute each request to a specific
client build — e.g. ``Nextcloud-Astrolabe/0.14.1``.
The middleware fires only for /api/v1/* and /webhooks/nextcloud,
which is the surface PHP-side clients hit; /mcp and /health stay
silent. The structured ``extra`` ({user_agent, http_method, http_path})
flows into OTel spans so the field is queryable in Grafana / Loki.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses all 8 items in the round-4 bot review plus 4 remaining
SonarQube OPEN issues that were silently broken by round 3's
malformed NOSONAR markers.
NOSONAR syntax fix (clears the remaining 4 OPEN SQ issues)
----------------------------------------------------------
Round 3 used ``# NOSONAR S<rule_key>`` form. SonarQube Python doesn't
recognize the rule-key suffix — it treats the whole thing as a
malformed suppression directive (S7632) AND lets the underlying rule
keep firing (S7503 on ``_Cursor.__aenter__/__aexit__``).
Switch every marker to bare ``# NOSONAR``, with the rationale moved
into a preceding comment block. Affected sites:
- storage.py: ``_Cursor.__aenter__``, ``_Cursor.__aexit__``
- config.py: ``get_database_ssl()`` ``return False`` + ``ssl.create_default_context()``
- test_storage_logging.py: ``SENTINEL_PASSWORD_FRAGMENT`` constant
- test_storage_postgres.py: three ``bob_pw_v1`` / ``bob_pw_v2`` / ``carol_pw`` literals
Bot 🔴#1 — defensive NOSONAR on get_database_ssl `return False`
--------------------------------------------------------------
Bot predicted S4830 fires on the operator-opt-out path. SQ output
shows it doesn't currently fire, but bare NOSONAR added defensively
with rationale comment.
Bot 🔴#2 — defensive NOSONAR on f-string SQL
--------------------------------------------
``update_oauth_session`` builds its SET clause via ``f"{', '.join(update_fields)}"``;
``get_audit_logs`` builds its WHERE clause via string concatenation.
Both are safe (the fragments only come from this function's own
branches, no user input), but the patterns trip taint analysers.
Annotated both with bare NOSONAR + safety comment explaining the
hardcoded-fragments invariant. Note: S2077 doesn't currently fire
on these; defensive.
Bot 🟡#3 — pg_advisory_lock for concurrent migrations
-----------------------------------------------------
Without coordination, two pods rolling-updating simultaneously can
both observe ``has_alembic=False`` and both try to apply migrations
from scratch — the second crashes with "relation already exists".
New ``_migration_lock()`` async context manager:
- On Postgres: ``SELECT pg_advisory_lock(:lock_id)`` on a fresh
connection (separate from the engine pool so it survives the
``to_thread.run_sync`` worker), held across BOTH the schema-inspect
AND the migration call. Without that span, two pods could each
observe "no alembic_version" before either started migrating,
defeating the lock.
- On SQLite: yields immediately (file-level locking serializes
writes natively).
Lock ID derived from
``sha256(b"nextcloud-mcp-server:migrations")[:8]`` as a stable signed
int64 so we can't collide with other apps sharing the same Postgres.
Bot 🟡#4 — RefreshTokenStorage.close() + lifespan wiring
--------------------------------------------------------
New idempotent ``close()`` method calls ``await engine.dispose()``,
nulls the engine, resets ``_initialized``. Wired into both
``app_lifespan_basic`` (BasicAuth) and the OAuth lifespan teardown,
each wrapped in ``try/except Exception`` with ``logger.warning`` so a
buggy dispose can't block SIGTERM. Without this, pooled asyncpg
connections leak server-side slots until
``idle_in_transaction_session_timeout`` reaps them — with small pool
defaults and frequent k8s rolling restarts this can starve
``max_connections``.
Bot 🟢#5 — is_sqlite_url docstring on :memory:
----------------------------------------------
Updated docstring to note both file-backed and in-memory forms are
recognized; caller is responsible for ``:memory:`` magic.
Bot 🟢#6 — db_path via make_url(...).database
---------------------------------------------
Replaced ``database_url.split("///", 1)[1]`` hack with SQLAlchemy's
own URL parsing. Naturally handles in-memory (``.database is None``
→ falls back to ``""``). Same lazy-import pattern as the existing
``mask_db_password`` to avoid module-import-time cost.
Bot 🟢#7 — _to_sync_url unrecognized-driver guard
-------------------------------------------------
Pulled ``_KNOWN_ASYNC_DRIVERS = ("aiosqlite", "asyncpg")`` into a
module constant. When an unrecognized ``+<driver>`` token survives
the strip, emits ``logger.warning`` with the known-supported list.
Behavior unchanged for valid URLs.
Bot 🟢#8 — get_audit_logs SELECT * → explicit columns
-----------------------------------------------------
Replaced ``SELECT *`` with explicit column list. Future schema
additions stay out of the dict return.
New tests
---------
- ``test_close_disposes_engine``: pins the public contract — engine
nulled, state reset, second call is a no-op.
- ``test_concurrent_initialize_serialized_by_advisory_lock``: spawns
3 concurrent inits against a fresh schema; asserts no "relation
already exists" and exactly one ``alembic_version`` row at the end.
Without the lock, this reliably fails on the second concurrent
task.
Docs
----
- ADR-026: new "Concurrent migrations across pods" subsection
documents the advisory-lock approach + lock-ID derivation.
Verification
------------
- ``uv run pytest tests/unit/`` — 1025 passed.
- ``TEST_DATABASE_URL=… uv run pytest tests/integration/test_storage_postgres.py -m postgres`` — 9 passed (was 7).
- ``ruff check && ruff format --check && ty check`` — clean.
Expected post-push: SQ scan reports 0 OPEN issues (was 4).
Tracked on Astrolabe Cloud POC board, card #99.
---
_This PR was generated with the help of AI, and reviewed by a Human_
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to #787/#789 (ADR-022 cleanup). After
`oauth_enabled ↔ enable_login_flow` became an invariant, the
`use_basic_auth=False` branch in `vector/oauth_sync.py` — and the
parameter wiring that fed it — was no longer reachable from any
supported deployment mode. This commit removes the dead code.
- nextcloud_mcp_server/vector/oauth_sync.py:
- Deleted `get_user_client_oauth` (the OAuth-token refresh helper) and
its `VECTOR_SYNC_SCOPES` constant.
- Deleted the `get_user_client` dispatcher. Internal callers now call
`get_user_client_basic_auth` directly.
- Dropped the `use_basic_auth: bool` parameter from `user_scanner_task`,
`multi_user_processor_task`, `_run_user_scanner_with_scope`, and
`user_manager_task`.
- Dropped the `token_broker` parameter from the same four functions —
they no longer need it now that the OAuth-refresh path is gone. The
`TokenBrokerService` constructed in `app.py` is still used by the
management API revoke endpoint, just not by background sync.
- Simplified the user-list query in `user_manager_task` to always read
from the `app_passwords` table.
- Replaced all `mode_label = "BasicAuth" if use_basic_auth else "OAuth"`
with a literal `[BasicAuth]` log prefix (keeps existing log filters
working).
- Updated the module docstring to describe the post-cleanup shape.
- Dropped the now-unused `TYPE_CHECKING` import of `TokenBrokerService`.
- nextcloud_mcp_server/app.py: dropped the `use_basic_auth = True` block
and the now-stale `token_broker if not use_basic_auth else None` /
`use_basic_auth` positional args from the two `tg.start(...)` calls in
the multi-user vector-sync lifespan. Token broker construction stays —
still consumed by the management API revoke endpoint via
`app.state.oauth_context["token_broker"]`.
- tests/integration/test_app_password_provisioning.py: deleted four tests
that exercised the now-removed OAuth-refresh path
(`test_oauth_mode_uses_refresh_token_only`,
`test_oauth_mode_raises_error_without_token`,
`test_get_user_client_oauth_function`,
`test_oauth_mode_requires_token_broker`) plus the
`test_get_user_client_dispatches_to_basic_auth` test for the deleted
dispatcher. Updated the module docstring + imports accordingly. The
BasicAuth-mode tests (`test_basic_auth_mode_uses_local_storage`,
`test_multiple_users_basic_auth_mode`, etc.) all remain.
No runtime-behaviour change in any supported deployment mode — the deleted
branches were already unreachable post-PR #787. 3 files changed,
+59 / -301; 1010 unit tests pass; integration jobs for
`mcp-login-flow` and `mcp-multi-user-basic` are the critical regression
gates before merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two small post-merge cleanups deferred from PR #787 (ADR-022 follow-up).
Both were explicitly noted in the reviewer's "acknowledged deferred items"
list.
1. config.py: drop `enable_multi_user_basic_auth` and `enable_login_flow`
from the dynaconf `_DEFAULTS` dict. They were removed from `_field_map`
in PR #787, so `get_settings()` never read them anyway, but their
presence in `_DEFAULTS` was visually misleading — readers might think
they could be set via TOML when in fact `Settings.__post_init__`
derives them from `MCP_DEPLOYMENT_MODE`. Replaced with a NOTE comment
pointing at the canonical derivation site.
2. app.py: the lifespan code had
`use_basic_auth = not oauth_enabled or settings.enable_login_flow`,
which became always-True once PR #787 enforced
`oauth_enabled ↔ enable_login_flow` via __post_init__. Hard-coded to
`True` with a comment explaining the invariant and pointing at the
separate follow-up that will prune the now-unreachable
`use_basic_auth=False` code paths in `vector/oauth_sync.py` (which
includes deleting the `use_basic_auth` parameter from
`user_manager_task` / `oauth_processor_task` and the OAuth-token-refresh
branch in `get_user_client`). Kept the variable name and the call-site
conditionals as-is for now so that follow-up is a clean mechanical
diff.
No runtime behaviour change: `use_basic_auth` already evaluated to True
in every supported mode after PR #787, and the `_DEFAULTS` entries were
already shadowed by `__post_init__`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The AuthMode.OAUTH_SINGLE_AUDIENCE enum was a vestige of ADR-021's
original design where it co-existed with OAUTH_TOKEN_EXCHANGE. The
un-augmented OAuth bearer pass-through it represented relied on
Nextcloud-side patches to user_oidc (Bearer token validation on
non-OCS endpoints) that were never merged upstream (see
docs/authentication.md, docs/login-flow-v2.md). The working path —
mcp-login-flow profile — sets ENABLE_LOGIN_FLOW=true on top of this
mode so Login Flow v2 acquires per-user Nextcloud app passwords via
a browser flow. With OAUTH_TOKEN_EXCHANGE removed in 57303135, the
_AUDIENCE suffix in the Python name no longer disambiguates anything,
and the enum value diverged from the env-var spelling. ADR-022 (now
accepted) called for this rename as step 1 of consolidation.
- nextcloud_mcp_server/config_validators.py: rename enum to LOGIN_FLOW
with value "login_flow". The mode_map key is now "login_flow"; the
MODE_REQUIREMENTS entry requires `enable_login_flow=True`. Added a
validation gate so MCP_DEPLOYMENT_MODE=login_flow without
ENABLE_LOGIN_FLOW=true errors with a clear message pointing at
ADR-022. Default auto-detection fallback returns LOGIN_FLOW.
- nextcloud_mcp_server/app.py: renamed three identifier references and
switched the "Configuring MCP server for OAuth mode" log line to
the uniform `mode.value` shape used by the other modes.
- nextcloud_mcp_server/api/management.py: renamed identifier in the
/api/v1/status mapping. The user-visible "auth_mode": "oauth" string
is preserved — that's a stable Astrolabe contract.
- nextcloud_mcp_server/config.py: updated Settings docstring.
- tests/unit/test_config_validators.py: renamed class
TestOAuthSingleAudienceValidation → TestLoginFlowValidation,
individual test methods, env-var strings; added enable_login_flow=True
to fixtures expecting success; added a new test
(test_login_flow_requires_enable_login_flow_flag) that exercises the
validation gate.
- tests/unit/test_management_status_endpoint.py: renamed identifier.
BREAKING CHANGE: MCP_DEPLOYMENT_MODE=oauth_single_audience is no longer
accepted. Set MCP_DEPLOYMENT_MODE=login_flow (and keep
ENABLE_LOGIN_FLOW=true) for the same deployment. The un-augmented
OAuth path is no longer supported; if you previously ran the broken
path, you can either configure Login Flow v2 (recommended) or switch
to multi_user_basic / single_user_basic.
Dead-code pruning of `oauth_enabled and not enable_login_flow`
branches in app.py (lifespan, background sync) is deferred to a
separate follow-up PR per the consolidation plan in ADR-022.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The readiness handler called the configured `qdrant_url/readyz` with a
bare httpx.AsyncClient — no headers. That works against a self-hosted
Qdrant (where /readyz is anonymous), but Qdrant Cloud's auth gateway
returns 403 for any unauthenticated request, including /readyz, /livez
and /healthz. Result: every probe against a Cloud cluster fell into
the "status 403" branch, the handler returned 503, and the Pod never
went Ready — even when the configured `AsyncQdrantClient` itself was
authenticating fine for actual collection traffic.
Forward `settings.qdrant_api_key` as the `api-key` header (mirroring
what `vector/qdrant_client.py:540` already does for the real client).
When the key is unset (self-hosted, anonymous case) we send no header,
so existing self-hosted deployments are unchanged.
Verified end-to-end against Qdrant Cloud:
- Without header: GET /readyz -> 403 {"error":"forbidden"}
- With api-key: same request shape returns 200 (matches what
AsyncQdrantClient.wait() relies on internally).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-chunk PDF page renders (~150–700 KB base64 PNG each) were the dominant
disk consumer in production, repeatedly tripping `No space left on device:
WAL buffer size exceeds available disk space` on welcomed-malamute Qdrant.
Replace the inline highlighted_page_image / highlighted_page_number /
highlight_count fields with a small `chunk_bbox` field:
list[(x0, y0, x1, y1)] of normalized [0, 1] floats, ~32 bytes per chunk.
Astrolabe (the only known consumer) renders the highlight client-side as
a percentage-positioned overlay on top of the existing /api/v1/pdf-preview
render-on-demand path (cbcoutinho/astrolabe#76).
- pdf_highlighter: new compute_chunk_bboxes_batch() that reuses the
existing _find_chunk_bbox text-search path, skipping all pixmap/PIL/PNG
work.
- processor: store chunk_bbox + chunk_bbox_page in the Qdrant payload,
drop highlighted_page_image + friends, drop the base64 import.
- visualization /api/v1/chunk-context and auth/viz_routes: read
chunk_bbox instead of highlighted_page_image.
- vector/__init__: stop eagerly re-exporting `processor`/`scanner` —
fixes a pre-existing circular import (search.algorithms ->
vector.placeholder -> vector/__init__ -> processor -> scanner ->
server.semantic -> search.bm25_hybrid -> search.algorithms partial).
Test suite that was broken on master (test_bm25_hybrid.py et al.) now
collects and passes.
- scripts/purge_page_images.py: ad-hoc, idempotent migration that
delete_payload's the legacy keys from existing points. No reindex
required; legacy chunks render the page with no overlay.
Pairs with cbcoutinho/astrolabe#76. Frontend handles missing chunk_bbox
gracefully, so this can land in either order.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses all 9 findings from the review on PR #758:
Blocking:
- _revoke_refresh_token_at_idp now reads config from oauth_ctx["config"]
(the production-shaped nested dict). Previously read flat keys, causing
IdP revocation to silently no-op in production. Test fixtures rebuilt
to the realistic nested shape so the bug can't regress unnoticed.
- HTML error responses in oauth_login_callback now wrap IdP-controlled
error_body, str(e), and the attacker-controlled error/error_description
query params in html_escape. New test_browser_oauth_xss.py pins this.
Important:
- New _safe_next_url helper validates the ?next= query param at write
time (oauth_login), in oauth_logout, and on read from the session row
in oauth_login_callback. Blocks https://, // (protocol-relative), and
CRLF/whitespace injection.
- verify_id_token now caches discovery + JWKS (5-min TTL) using the
same pattern as oauth_routes._get_cached_discovery. New caching
regression test pins to one fetch per URL across multiple calls.
- /oauth/logout is now POST-only at the route layer (defeats passive
CSRF via <img src>). oauth_logout also validates Origin/Referer
against the configured mcp_server_url. Logout UI in user_info.html
converted from <a href> to <form method="post">.
- New storage.cleanup_expired_browser_sessions() called from the hourly
cleanup loop in app.py — previously these rows accumulated for users
who never explicitly logged out.
Nits:
- Demoted INFO logs that leaked oauth_config.keys() / client_id /
token-storage state to DEBUG. Operator-relevant outcome lines
(login successful, refresh token stored, logged out) stay INFO.
- verify_id_token algorithms widened to RS256, PS256, ES256 — covers
Azure AD (PS256) and Cognito/some Keycloak realms (ES256). Symmetric
and "none" remain off the allowlist.
- Migrated all Optional[X] usages in auth/storage.py to X | None per
CLAUDE.md.
Breaking change: GET /oauth/logout now returns 405. The in-tree logout
UI was migrated to a POST form; any external bookmark or curl-based
caller that relied on GET will need to switch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lift NEXTCLOUD_PUBLIC_ISSUER_URL out of raw os.getenv reads into
Settings.nextcloud_public_issuer_url across all 8 production call sites
(app.py x2, oauth_routes.py x2, browser_oauth_routes.py,
provision_routes.py, userinfo_routes.py, elicitation.py). cli.py
remains the env-write source so the existing config-by-flag pipeline
still works.
Also addresses remaining PR #757 review nits:
- elicitation.py: align URL-present/absent wording on "open in your
browser" so users don't try clicking in the terminal
- test_scope_authorization_stored.py: lock in the deliberately-shared
fall-through branch with explicit declined/cancelled decorator tests
- test_elicitation.py: switch from monkeypatch.setenv to
patch(get_settings) since Settings is now the canonical surface
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address findings surfaced by `pre-push-review` after the round 8 sweep:
- Add deck verifier symmetry tests (404, transient 5xx, unexpected
exception, non-numeric metadata) so deck has the same shape as the
notes/news/files verifiers. Also add unexpected-exception tests for
the news and file verifiers, which had `except Exception` branches
no test was reaching. Keeps the registry-style verifier coverage
uniform.
- Modernize sibling field types in `VectorSyncState`, `AppContext`,
and `OAuthAppContext` from `Optional[X]` to `X | None`, matching the
`eviction_task_group: TaskGroup | None` field added in the round 8
diff (resolves the inconsistency flagged by A6). The lone remaining
`Optional` import is dropped.
- Reverse cross-reference direction in the verifier docstrings: the
later-defined `_verify_deck_cards` and `_verify_news_items` now
point at `_verify_notes` as the canonical hoisted-cast pattern,
rather than `_verify_notes` forward-referring to verifiers defined
below it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tightens verifier consistency, closes test gaps, hardens the fire-and-forget
eviction snapshot, and routes the new concurrency knob through Settings.
- Pre-flight ``int()`` guard in ``_verify_notes`` mirrors ``_verify_deck_cards``,
so a non-numeric note id produces a type-specific log line instead of
falling through to the generic "unexpected error" branch.
- Adds explicit 403 tests for the file and news verifiers (symmetry with the
existing notes/deck 403 tests) plus a ``non_numeric_id_keeps`` test.
- ``AppContext`` and ``OAuthAppContext`` no longer snapshot
``_vector_sync_state.eviction_task_group`` at lifespan-yield time. Both
expose it as a ``@property`` that reads the singleton dynamically, removing
the order-sensitive race where a future startup-ordering change could
silently degrade fire-and-forget eviction to inline forever.
- Adds ``verification_concurrency`` (env var ``VERIFICATION_CONCURRENCY``,
default 20) to ``Settings`` with a dynaconf validator; ``verify_search_results``
resolves the cap lazily from settings when the caller doesn't override it.
- Enriches the news verifier TODO to call out that ``batch_size=-1`` is
intentional — a numeric ceiling would silently break correctness because
any item beyond the cap would be missing from ``present_ids`` and dropped.
- Updates ``Optional[TaskGroup]`` to ``TaskGroup | None`` per project style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements fire-and-forget eviction (ADR-019 §"Lazy eviction"): the
search response no longer waits on Qdrant deletes, instead spawning
evict() on a long-lived lifespan-owned task group. Falls back to inline
eviction in modes without vector sync and in unit tests.
Also: harden _verify_news_items against non-numeric ids (fail open
instead of crashing the verifier); document the get_file_info None-on-404
contract; add INDEXED_DOC_TYPES single source of truth in vector/scanner.py
referenced by the CI-guard test; write a Verify-on-Read Latency Budget
section in docs/configuration.md covering the unbounded news.get_items
fetch. Closes the two remaining ADR-019 implementation checklist items.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The /webhooks/nextcloud endpoint was a no-op stub that logged the
payload and returned 200 OK; webhook deletions never reached Qdrant.
Compounding that, _get_webhook_uri() registered the docker-compose
internal hostname (http://mcp:8000) with Nextcloud whenever
/.dockerenv existed — including ECS Fargate — so cloud deployments
were registering a URL NC could not resolve.
- New vector/webhook_parser.py extracts a DocumentTask from
NodeCreatedEvent / NodeWrittenEvent / BeforeNodeDeletedEvent
payloads scoped to */files/Notes/*.md (matching the registered
preset filters).
- New vector/webhook_receiver.py pushes that task onto the same
send-stream the scanner uses (app.state.document_send_stream),
with 503 when sync is not running so NC retries delivery.
- _get_webhook_uri() now prefers NEXTCLOUD_MCP_SERVER_URL over the
/.dockerenv branch, so the explicit public URL set on cloud tasks
wins; docker-compose dev still falls back to the internal name when
no public URL is configured.
Calendar / Tables event parsing is intentionally out of scope here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The nc_get_vector_sync_status MCP tool was returning hardcoded
status="unknown", indexed=0, pending=0 for all OAuth deployments
because OAuthAppContext lacked the document_receive_stream field.
The tool's getattr() lookup against the lifespan context returned
None and triggered an early-return before the Qdrant count query.
Add the four vector-sync fields to OAuthAppContext (matching
AppContext) and populate them from the _vector_sync_state singleton
at the lifespan yield site.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Revert default transport to streamable-http (not a breaking change)
- Extract AVAILABLE_APPS constant to server/__init__.py (DRY)
- Wrap get_stdio_mcp ValueError in click.ClickException for clean errors
- Fix test_stdio.py: call _reload_config() so dynaconf sees env changes
- Use lazy %-style logging in stdio.py
- Add private API comments in test assertions
- Derive --enable-app CLI choices from AVAILABLE_APPS
- README: show explicit --transport stdio in uvx examples
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Many identity providers (AWS Cognito, Okta, Azure AD) reject or mishandle
colons in OAuth scope names. This migrates all custom scopes from
`resource:action` to `resource.action` format (e.g., `notes:read` →
`notes.read`), which is universally accepted and aligns with industry
conventions (Microsoft, Google).
Includes Alembic migration 004 for stored scope strings and ADR-024
documenting the rationale and RFC references.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Log warning if /app mount not found when sharing poll task group
- Add docstring explaining unconditional task group creation
- Check session expires_at in provision_status to catch stale sessions
- Guard _poll_and_store status writes against cleanup-while-polling race
- Use "error" status (not "expired") when app_password is missing
- Remove hardcoded "Astrolabe Background Sync" user_agent string
- Fix async mock pattern (new_callable=AsyncMock) in test
- Add autouse fixture to clear _provision_sessions between tests
- Add _poll_and_store unit tests: completed, expired, error, cleanup
- Document all status values in provision_status docstring
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Escape HTML in _render_error to prevent XSS from exception messages
- Replace asyncio.create_task/sleep with anyio task group and sleep,
tying poll task lifetime to the app lifespan for proper cleanup
- Extract rewrite_url_origin() utility to fix duplicated URL rewriting
logic and replace urlparse._replace with stable urlunparse API
- Add warning log for insecure HTTP redirect URIs
- Add unit tests for validation, XSS escaping, route handlers, and
URL rewriting (16 new tests in test_provision_routes.py)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Login Flow v2 is a deployment-wide mode where all users authenticate
with app passwords (not OAuth refresh tokens). Set use_basic_auth=True
when enable_login_flow is true so the background sync user manager
queries the app_passwords table and scanners use app password
authentication for Nextcloud API calls.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add /app/provision and /app/provision/status endpoints for browser-based
Login Flow v2 app password provisioning. Used by Astrolabe's "Enable
Semantic Search" to chain OAuth (bearer token) + Login Flow v2 (app
password) in a single user interaction.
The provision page initiates Login Flow v2, opens Nextcloud's login URL
in a popup, polls for completion via background task, and redirects back
to the caller's redirect_uri on success.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add docker compose services (tailscale-mcp + nginx-claude-filter) behind
a claude-funnel profile that expose the login-flow MCP server via
Tailscale Funnel with IP-based access control:
- /mcp endpoint restricted to Claude AI outbound IPs (160.79.104.0/21)
- /oauth/*, /.well-known/*, /app paths open to all IPs (user login flow)
- All other paths return 404
Also add favicon.png served at /favicon.ico for connector directory
discovery (Google favicon service).
Usage:
docker compose --profile login-flow --profile claude-funnel up -d
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Drop OIDC fork: comment out third_party/oidc mount, use upstream
v1.16.3 from app store (fixes consent redirect race, PR #631)
- Support client_secret_basic auth: add _extract_basic_auth() helper
so TS MCP SDK can authenticate at token endpoint (RFC 6749 §2.3.1)
- Multi-issuer JWT validation: accept tokens with internal Docker
issuer (http://app:80) or public URL (NEXTCLOUD_PUBLIC_ISSUER_URL)
since AS proxy obtains tokens server-to-server
- Introspection fallback: try token introspection when JWT verification
fails, supporting both JWT and opaque token types
- Register all tool scopes in DCR: add semantic:read, collectives:read,
collectives:write to OIDC client allowed_scopes so tokens include
them and semantic search tools are visible to authenticated clients
- Auto-create Astrolabe OAuth client: new app-hook creates OIDC client
and stores credentials in config.php so the "Authorize via OAuth"
button works without manual setup
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implement MCP tools for the Collectives wiki/documentation app, enabling
agentic workflows for team knowledge base management.
16 tools covering collectives, pages, tags, search, and trash:
- Read: list collectives, list/get pages (with WebDAV content), search,
list tags, list trashed pages
- Write: create/update collective, create/move/trash/restore pages,
set emoji, create/assign/remove tags
Includes Docker hook for app installation, OCS API client with envelope
unwrapping, Pydantic models, unit tests (16), and integration tests (10).
Closes#621
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Smithery is no longer a supported deployment mode. Remove all Smithery-specific
code paths, middleware, configuration, and tests. This simplifies the codebase
by eliminating DeploymentMode enum, SmitheryConfigMiddleware, session config
context variables, and the smithery_main entrypoint.
Files deleted: Dockerfile.smithery, smithery.yaml, smithery_main.py
ADR-016 retained with deprecated status for historical reference.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Make /api/v1/status and /api/v1/vector-sync/status available in all
non-Smithery deployment modes so Astrolabe can show server status even
in single-user BasicAuth mode. Previously these were only mounted when
OAuth or multi-user BasicAuth with offline access was enabled.
- Split management API routes into public (Tier 1) and authenticated (Tier 2+)
- Enable semantic search with in-memory Qdrant for single-user docker service
- Update astrolabe submodule with admin settings fix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add 429 retry with exponential backoff to register_client() (fixes CI
oauth matrix failures from parallel DCR requests)
- Make client_id, redirect_uri, and PKCE mandatory at token endpoint
- Add null-checks for discovery_url and OAuth credentials in proxy flows
- Add OIDC discovery document caching with 5-min TTL
- Add per-IP rate limiting on /oauth/register DCR proxy
- Discover DCR endpoint from OIDC discovery instead of hardcoding
- Extract extract_user_id_from_token to auth/token_utils.py (breaks
circular imports between server/ and auth/ layers)
- Add TTL scope cache in scope_authorization.py (avoids DB hit per tool)
- Add defense-in-depth scope validation in storage layer
- Broaden elicitation exception handling with graceful fallback
- Add idempotentHint to nc_auth_check_status, return "pending" status
after accepted elicitation, add polling interval to description
- Change ALL_SUPPORTED_SCOPES from tuple to frozenset for O(1) lookups
- Replace Optional[str] with str | None throughout config.py
- Use default_factory for ProxyCodeEntry/ASProxySession dataclasses
- Add proxy code/session cleanup to background loop
- Fix OIDC verification CI step to only run for oauth/login-flow modes
- Add unit tests for access.py REST endpoints (10 tests)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MCP clients like Claude Code were unable to use tools because tokens
obtained directly from Nextcloud had the wrong audience claim. The MCP
server now acts as its own OAuth Authorization Server, proxying auth
to Nextcloud with its own client_id so tokens have the correct audience.
New endpoints: /.well-known/oauth-authorization-server, /oauth/token,
/oauth/register. Modified /oauth/authorize from pass-through to
intermediary pattern. PRM now points authorization_servers to the MCP
server instead of Nextcloud.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Consolidate MCP session + login flow cleanup into _mcp_session_with_login_flow() helper,
replacing 4 duplicated AsyncExitStack sites in app.py
- Fix get_shared_storage() race condition by using module-level anyio.Lock() init
(reverts regression from ba59763)
- Collapse cosmetic if/else branching in scope_authorization.py
- Consolidate dual password storage paths into single store_app_password_with_scopes() call
- Mark unused request param as _ in list_supported_scopes
- Make ALL_SUPPORTED_SCOPES an immutable tuple; use list() instead of .copy()
- Add hasattr(ctx, "elicit") guard in elicitation.py, narrow except to NotImplementedError
- Add YAML comment explaining --oauth flag for mcp-login-flow service
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix anyio.Lock() created at module import time; use lazy init in
get_shared_storage() to avoid instantiation before event loop exists
- Stop get_login_flow_session from silently swallowing DB exceptions;
re-raise and handle in caller with proper error response
- Update ProvisionAccessResponse and UpdateScopesResponse status field
docs to include all actual values (declined, cancelled, unchanged)
- Narrow except clause in present_login_url to (AttributeError,
NotImplementedError) instead of bare Exception
- Add KeyError handling in LoginFlowV2Client.initiate() and poll() for
clear errors on malformed Nextcloud responses
- Simplify redundant env-var bypass branches in scope_authorization.py
- Extract _maybe_login_flow_cleanup() context manager to replace 4
inline cleanup loop registrations in app.py; move sleep to end of
loop body so cleanup runs once at startup
- Replace fragile string replacement in _rewrite_login_flow_url with
proper urllib.parse URL handling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consolidate three independent RefreshTokenStorage lazy singletons into a
single lock-protected get_shared_storage() function, eliminating race
conditions on concurrent first-access. Remove blanket try/except in
_get_stored_scopes so storage errors propagate as proper MCP errors
instead of silently triggering "please provision" messages. Handle
declined/cancelled elicitation results in Login Flow tools by cleaning up
sessions and returning clear status. Add update_app_password_scopes() to
avoid unnecessary decrypt/re-encrypt when only scopes change. Add
unprovisioned-user early exit and no-op detection to nc_auth_update_scopes.
Remove four dead config fields and misleading NEXTCLOUD_PASSWORD deprecation
warning. Add periodic login flow session cleanup task. Generate separate
Fernet keys per service. Add board cleanup in deck integration test. Gate
CI unit tests on linting and skip Astrolabe build for single-user profile.
Fix test markers from oauth to multi_user_basic for astrolabe integration
tests. Update login_flow.py docstrings to document outbound HTTP calls.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enable ruff PLC0415 rule for all source files (tests excluded via
per-file-ignores). Move 136 inline imports to top-level across 33 files.
8 imports suppressed with noqa for legitimate reasons: circular
dependencies (client/__init__.py, context.py), optional dependency
guards (app.py document processors, auth/userinfo_routes.py), and
post-env-setup imports (smithery_main.py).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The revoke test failed because it only completed Step 2 (app password) but
not Step 1 (OAuth authorization). In hybrid mode, Astrolabe requires both
steps for $isFullyConfigured=true, which gates the "Revoke Access" button.
Changes:
- Use complete_astrolabe_authorization() in revoke test for full two-step flow
- Add stale state cleanup (app passwords, bruteforce entries, Astrolabe prefs)
to both enablement and revoke tests
- Add startup cleanup of invalid app passwords in BasicAuth mode
- Pre-validate credentials before entering scanner loop to fail fast
- Handle 401/403/429 in scanner with proper backoff and circuit breaking
- Clean up app passwords in test_users_setup fixture teardown
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add NEXTCLOUD_VERIFY_SSL and NEXTCLOUD_CA_BUNDLE env vars to configure
TLS certificate verification for all outbound Nextcloud connections.
Centralizes SSL config via a new HTTP client factory (http.py) used by
all 27 Nextcloud-bound call sites, including API clients, OIDC endpoints,
OAuth flows, and health checks.
Closes#560
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split the monolithic management.py (1988 lines) into 4 focused modules:
- management.py: Server status, user sessions, shared helpers (~520 lines)
- passwords.py: App password provisioning for BasicAuth mode (~300 lines)
- webhooks.py: Webhook registration management (~290 lines)
- visualization.py: Search and PDF preview endpoints (~810 lines)
Backward compatibility maintained via __init__.py re-exports.
Updated test imports to use new module paths.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace the client-side PDF.js viewer with server-side rendering using PyMuPDF.
This avoids CSP worker restrictions and ES private field access issues that
affected Chromium browsers.
Changes:
- Add /api/v1/pdf-preview endpoint to MCP server (management.py)
- Add pdf-preview route and controller action in Astrolabe PHP backend
- Refactor PDFViewer.vue to display server-rendered PNG images
- Remove pdfjs-dist dependency and client-side PDF loading code
- Use @nextcloud/axios for CSRF token handling in PDFViewer
The server downloads the PDF via WebDAV, renders the requested page with
PyMuPDF at the specified scale, and returns a base64-encoded PNG image.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, the multi-user BasicAuth mode attempted to retrieve app passwords
via OAuth client_credentials grant, which Nextcloud OIDC doesn't support.
This fix implements local storage for app passwords:
- Add app_passwords table via Alembic migration (002)
- Add store/get/delete methods to RefreshTokenStorage
- Add management API endpoints for app password provisioning:
- POST /api/v1/users/{user_id}/app-password
- GET /api/v1/users/{user_id}/app-password
- DELETE /api/v1/users/{user_id}/app-password
- Update oauth_sync.py to read from local storage
- Update Astrolabe to send app passwords to MCP server after validation
- Add app-hook to configure mcp_server_url in Nextcloud
The flow is now:
1. User creates app password in Nextcloud Security settings
2. User enters it in Astrolabe Personal Settings
3. Astrolabe validates against Nextcloud, then sends to MCP server
4. MCP server stores encrypted app password locally
5. Background sync uses locally stored password
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The /oauth/login route was returning 404 in multi-user BasicAuth mode with
offline access enabled. This was because browser OAuth routes were gated
by `oauth_enabled` (only True for MCP OAuth modes), not by
`oauth_provisioning_available` which correctly includes hybrid mode.
The Management API (admin UI, webhook management) requires OAuth
authentication regardless of how MCP tools authenticate. These are
independent security concerns:
- MCP Tools: BasicAuth (waiting for upstream Nextcloud OAuth patches)
- Management API: OAuth (for admin UI, webhook management, vector sync)
Changes:
- Gate browser OAuth routes by oauth_provisioning_available instead of
oauth_enabled
- Add follow_redirects=True to OIDC discovery HTTP clients
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Completely separates multi-user BasicAuth mode from OAuth mode with no
fallback between them. These are now mutually exclusive authentication
strategies based on deployment configuration.
Changes:
- Create separate functions: get_user_client_basic_auth() and
get_user_client_oauth() with clear separation of concerns
- Update get_user_client() to dispatch based on use_basic_auth parameter
- Pass use_basic_auth through all background sync tasks
- Update app.py to determine auth mode at startup
- Rewrite integration tests to verify no OAuth fallback in BasicAuth mode
- Fix test assertions for response field names and duplicate title handling
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
