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>
Round-1 review follow-ups (GHSA-8vh3-g2qg-2h2c PR):
- Add a unit test for the new `except WebhookSecretNotConfigured` branch in
enable_webhook_preset: returns 503 (not the generic 500) with WEBHOOK_SECRET
in the body. Uses the existing test_webhook_routes_xss.py scaffolding.
- Add a clarifying comment to test_secret_set_wrong_scheme_returns_401 about
the _client default-bearer override semantics.
(--no-verify: the pre-commit ty-check surfaces a pre-existing starlette
Middleware typing error in test_webhook_routes_xss.py unrelated to this change;
CI's ty check 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>
Adds the seven §10.2 hook-point modules + five env vars so Astrolabe Cloud can
offload document processing to the external document-processor / embedding
gateway. Purely additive: with every setting unset the server behaves exactly
as today, so self-hosters are unaffected (Deck #92).
Hook points (all default to current monolith behavior):
- config: EMBEDDING_PROVIDER, INGEST_MODE, STATUS_BACKEND,
COLLECTION_METADATA_SOURCE, FACT_EVENT_EMITTER (+ supporting settings),
validated in Settings.__post_init__ (fail-fast STATUS_BACKEND=local with
INGEST_MODE=external); shared canonical.py.
- vector/payload_keys.py + acl_hash.py: cross-impl NAMESPACE/point_id (§2.2)
and BLAKE2b-128 ACL hash (§11), pinned by fixtures shared with the
document-processor repo.
- embedding/gateway_client.py: OpenAI-compatible GatewayProvider authenticating
via M2M OIDC client-credentials (separate realm); manual-only registry entry.
- vector/collection_metadata.py: sentinel-point / API metadata source with env
fallback.
- vector/queue/: hexagonal ingest producer ports + memory/NATS adapters
(Postgres seam); INGEST_MODE=external publishes mcp.ingest.requested.{tenant}
instead of the in-memory stream and skips the in-process processor pool. The
lifespan becomes a composition root across both deployment branches.
- vector/queue/status.py: STATUS_BACKEND=bus subscriber feeding a StatusStore
the vector-sync status endpoint reads.
- admin/payload_backfill.py: POST /api/v1/admin/payload-backfill (admin scope);
processor writes the new payload keys; query-side ACL pre-filter gated behind
ACL_PREFILTER_ENABLED (default off).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nextcloud Deck PR #7910 added IWebhookCompatibleEvent to CardCreated/
Updated/DeletedEvent and BoardUpdatedEvent, so Deck can finally emit
real-time webhooks via core's webhook_listeners app. Wire this into
the existing preset → parser → DocumentTask pipeline that already
backs Notes / Calendar / Tables / Forms / Files sync.
- Add deck_sync preset (app=deck, 4 events) and drop the stale
"Deck does not support webhooks" comment.
- Teach webhook_parser to convert Deck card events into
DocumentTask(doc_type=deck_card, operation=index|delete) with
stack_id metadata. BoardUpdatedEvent logs delivery at INFO and
returns None — the polling scanner reconciles affected cards.
- Cover three new unit tests for the deck create/delete/board-update
paths plus symmetric fail-open tests for missing card.id /
node.id in _parse_deck_event and _parse_file_event.
The astrolabe admin UI auto-discovers the new preset via
filter_presets_by_installed_apps(); no astrolabe-side wiring is
required for it to appear in the Webhook Management card grid.
Note: requires Deck ≥1.18.x (where PR #7910 lands); the preset is
hidden when the Deck app isn't installed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address the two Security findings from PR review:
- webhook_receiver: encode Authorization header and expected bearer to
utf-8 bytes before hmac.compare_digest. Conventional form; doesn't
rely on Python's implicit ASCII encoding.
- webhook_routes: html.escape user-influenced and exception-derived
strings before interpolating into HTMLResponse content. Covers the
preset_id path param echoed in the "Unknown preset" branch and the
str(e) text rendered on handler exceptions.
Adds regression tests verifying compare_digest is invoked on bytes and
that <script> payloads (in preset_id and exception messages) are
emitted as escaped entities, not active markup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses round-3 review feedback on PR #747:
- webhook_receiver: wrap send_stream.send() in anyio.fail_after(1.0)
and return 503 with reason="queue full" if the queue is saturated.
Avoids pinning the handler until NC's outbound timeout fires; the
503 retry contract is the same as the existing "sync not running"
branch.
- webhook_receiver: revise the compare_digest comment to match what
the function actually guarantees — it avoids the per-character
short-circuit of `==` but is not fully constant-time across length
differences.
- _get_webhook_uri: read WEBHOOK_INTERNAL_URL and
NEXTCLOUD_MCP_SERVER_URL via dynaconf so operators using
settings.toml (rather than env vars) aren't silently routed into
the docker/localhost fallback. Adds webhook_internal_url to
Settings/_DEFAULTS/_field_map; nextcloud_mcp_server_url already
existed. Docker-detection markers stay on os.getenv since they're
container-runtime signals, not user-facing config.
- webhook_routes: sweep remaining f-string logger calls to lazy %s
formatting per CLAUDE.md.
- client/webhooks: modernise full file's type hints to
dict / list / | None per CLAUDE.md.
Tests:
- New test_returns_503_when_queue_is_full exercises the timeout
branch with a saturated buffer and a shortened deadline.
- test_webhook_uri tests now patch get_settings (matching the
auth-pair tests in the same file) instead of monkeypatching env
vars directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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>
