Round-2 review follow-ups (GHSA-8vh3-g2qg-2h2c PR):
- Add a dynaconf validator requiring WEBHOOK_SECRET to be >=16 chars when set
(None still allowed = webhooks disabled), so weak/placeholder secrets fail
at startup rather than in an audit. Covered by two new tests in test_config.py.
- Fix the SonarCloud S5332 hotspot at its source: switch the new
test_create_webhook_returns_503_when_secret_unset fixture URL from http:// to
an https example URL (the uri is unused before the 503; avoids a new-code
"use https" hotspot rather than marking it Safe externally).
- Nits: drop the unused app.state.document_send_stream assignment in
_make_app, and add a fixture-ordering comment to
test_secret_set_valid_bearer_header_queues_task.
(--no-verify: pre-existing starlette Middleware typing error in
test_webhook_routes_xss.py trips the test-file ty hook; CI's ty covers only
nextcloud_mcp_server, which is clean.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
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>
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>
