Commit Graph
9 Commits
Author SHA1 Message Date
Chris CoutinhoandClaude Opus 4.8 4fc2b10945 fix(security): require WEBHOOK_SECRET for the Nextcloud webhook receiver
GHSA-8vh3-g2qg-2h2c (CVSS 9.1, CWE-306): POST /webhooks/nextcloud had no
authentication when WEBHOOK_SECRET was unset (the default). The receiver
trusted the attacker-supplied user.uid and fed it to Qdrant, letting an
unauthenticated network caller delete or re-index any user's vector
embeddings.

Webhooks now require WEBHOOK_SECRET end-to-end:

- app.py: the /webhooks/nextcloud route is only mounted when WEBHOOK_SECRET
  is set; otherwise it 404s and a startup warning notes vector sync falls
  back to the polling scanner.
- webhook_receiver.py: removed the warn-and-accept fallback. No secret -> 503,
  missing/invalid bearer -> 401; the payload is never processed unauthenticated.
- webhook_routes.py / api/webhooks.py: webhook_auth_pair() raises
  WebhookSecretNotConfigured instead of returning authMethod="none"; both
  registration entry points return a clear 503 so no dead unauthenticated
  webhooks are created.

Also expose webhooks availability to the Astrolabe UI via GET /api/v1/status
("webhooks_enabled": bool), set WEBHOOK_SECRET on the docker-compose
semantic-search dev services, and update env.sample + ADR-010 / ADR-018 /
webhook-management-guide docs.

Vector sync still works without a secret via the polling scanner.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-14 18:18:34 +02:00
Chris CoutinhoandClaude Opus 4.7 b3a7587f1a fix(webhooks): use HTTP 428 instead of 412 for unprovisioned users
428 (Precondition Required, RFC 6585) is the correct semantic — the
request requires the client to complete a prerequisite step (Login Flow
v2 provisioning) before retrying. 412 (Precondition Failed) is for
header-based preconditions like ETags / If-Match.

No behavior change beyond the status code; same JSON payload.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 00:15:55 +02:00
Chris CoutinhoandClaude Opus 4.7 a0e484d95b fix(webhooks): use app-password basic auth for NC API calls
The webhook API endpoints in api/webhooks.py forwarded the inbound MCP
OAuth bearer token directly to Nextcloud as the Authorization header.
Per ADR-022 / docs/login-flow-v2.md the data leg from MCP server to
Nextcloud must use HTTP Basic Auth with the user's stored Login Flow v2
app password — bearer-forwarding requires upstream user_oidc patches that
were never merged and is incompatible with admin endpoints gated by
@PasswordConfirmationRequired (e.g. webhook_listeners/api/v1/webhooks,
which 401s).

PR #760 papered over the symptom for /api/v1/apps by switching to the
permissive /cloud/capabilities endpoint, but the same architectural
mistake remained on list_webhooks / create_webhook / delete_webhook,
which still 500'd on the astrolabe admin UI's preset page.

Changes:
- New helper api/_auth.py:get_basic_auth_for_user(user_id) reads the
  user's app password from encrypted storage and returns
  (username, app_password). Mirrors context.py:_get_client_from_login_flow
  but is callable from Starlette routes (no MCP Context required).
- All four endpoints in api/webhooks.py now use httpx.BasicAuth instead
  of forwarding the OAuth bearer; ProvisioningRequiredError is mapped to
  HTTP 412 so callers can render a "complete provisioning" CTA rather
  than receiving an opaque 500.
- Outbound NC requests now identify the user by the username recorded at
  Login Flow v2 provisioning time (which may differ from the IdP-issued
  user_id) — flowed into WebhooksClient and used for logging.

Tests:
- tests/unit/test_management_apps_endpoint.py: assertions updated to
  verify outbound NC request uses BasicAuth and carries no Authorization
  header. Replaced "missing-Authorization → 500" test with a
  ProvisioningRequiredError → 412 case.
- tests/unit/test_webhooks_api_auth.py (new): cross-endpoint coverage
  for list_webhooks, create_webhook, delete_webhook and the new helper —
  including 412 symmetry for all four endpoints.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-03 23:50:59 +02:00
Chris CoutinhoandClaude Opus 4.7 148ab8c117 fix(webhooks): use OCS v2 capabilities for /api/v1/apps
The Astrolabe webhooks UI hits /api/v1/apps on the MCP server, which
forwarded the OAuth bearer token to /ocs/v1.php/cloud/apps?filter=enabled.
That OCS endpoint is admin-only AND @PasswordConfirmationRequired —
neither requirement is satisfiable via an OAuth bearer token, so even an
admin user's token returns a silent 401 (no entry in nextcloud.log).

Switch to /ocs/v2.php/cloud/capabilities, which has no admin or password-
confirmation gate, accepts the existing bearer token, and returns a
capabilities map keyed by app id (notes, files, tables, forms, etc.).
This is sufficient for the webhook presets UI to gate available presets
against the running Nextcloud instance's enabled apps.

Bearer is preserved on the outbound call because anonymous capabilities
omits notes/tables/forms — only authenticated capabilities exposes them.

Tests:
- New unit test covers the regression (asserts /ocs/v2.php/cloud/capabilities
  is hit, NOT /cloud/apps), response parsing, sanitized error messages,
  and missing-config paths.
- New integration test under tests/server/login_flow/ drives a real
  OAuth flow against mcp-login-flow with a static OIDC client
  (nextcloudMcpServerUIPublicClient) and asserts /api/v1/apps returns 200
  with core/files in the response.

docker-compose.yml: aligns mcp-login-flow's ALLOWED_MGMT_CLIENT with
mcp-multi-user-basic so the same static-client test fixture works for both.

Follow-up to homelab-argocd #1608, which set ALLOWED_MGMT_CLIENT in
production but didn't unblock the webhooks flow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-03 22:26:35 +02:00
Chris CoutinhoandClaude Opus 4.7 affe29f72e fix(webhooks): escape webhook_uri, lazy logging, document 401 header omission
Address round-5 reviewer feedback on PR #747:

- Escape `webhook_uri` in the admin pane HTML template so an operator-
  controlled env value (`WEBHOOK_INTERNAL_URL`, `NEXTCLOUD_MCP_SERVER_URL`)
  can't inject markup. The sibling `preset_id` and exception messages were
  already escaped — this one was the odd one out.
- Convert the eight remaining f-string `logger.warning`/`logger.error`
  calls in `api/webhooks.py` to lazy `%s` formatting, matching the style
  already adopted by `webhook_receiver.py` and `webhook_routes.py`.
- Document why the 401 from `handle_nextcloud_webhook` deliberately omits
  `WWW-Authenticate`: NC's webhook delivery worker has no auth-flow state
  machine to negotiate against, the bearer is a static shared secret
  configured out-of-band via `WEBHOOK_SECRET`, and a challenge response
  wouldn't change client behaviour. The existing warning log already
  records the rejection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-30 14:37:34 +02:00
Chris CoutinhoandClaude Opus 4.7 224428fca5 fix(webhooks): authenticate deliveries via WEBHOOK_SECRET; review nits
Adds optional shared-secret authentication for /webhooks/nextcloud,
addressing the security follow-up flagged in #747.

Behavior:
- WEBHOOK_SECRET set: registrations pass authMethod="header" with
  authData={"Authorization": "Bearer <secret>"} (encrypted at-rest in
  Nextcloud's DB and forwarded on every delivery). The receiver
  validates the same header with hmac.compare_digest before parsing
  any payload; missing/invalid → 401.
- WEBHOOK_SECRET unset: registrations stay on authMethod="none" and
  the receiver accepts unauthenticated POSTs (logging a one-time
  startup warning). Backward compatible — operators can roll out at
  their own pace.

Implementation notes:
- WebhooksClient.create_webhook gains an `auth_data` parameter mapped
  to NC's `authData` body field; this is distinct from the existing
  `headers` parameter (`headers` is plaintext static request headers,
  `authData` is encrypted at-rest in NC and only emitted when
  authMethod="header"). The previous `auth_method="bearer"` mention in
  the docstring was incorrect — NC supports only "none" and "header".
- A small `webhook_auth_pair()` helper in auth/webhook_routes.py
  centralises the secret→(auth_method, auth_data) resolution so the
  preset flow and the Astrolabe-facing /api/v1/webhooks endpoint stay
  in sync.

Also addresses the smaller review points from #747:
- f-string → lazy %s formatting in webhook_receiver.py and
  webhook_routes.py.
- Move `int(time)` inside webhook_parser's try/except so a malformed
  `time` field returns None instead of raising ValueError.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-30 03:50:28 +02:00
Chris CoutinhoandClaude Opus 4.6 a11ae9c027 refactor: enforce PLC0415 (import-outside-top-level) for source code
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>
2026-02-20 08:04:50 +01:00
Chris CoutinhoandClaude Opus 4.6 1707b2e6e1 feat: add self-signed SSL certificate support for Nextcloud connections
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>
2026-02-16 09:21:21 +01:00
Chris CoutinhoandClaude Opus 4.5 2e7774654b refactor(api): split management.py into domain-focused modules
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>
2026-01-26 21:28:18 +01:00