From b79d485ef1bcc09325426b507b11fe0d787777c5 Mon Sep 17 00:00:00 2001 From: NuklearRabbit <145918611+NuklearRabbit@users.noreply.github.com> Date: Tue, 4 Aug 2026 05:03:33 +0200 Subject: [PATCH] docs+fix: audit live n8n state, require auth on the return webhook Inspected the shared n8n instance (n8n.itworx.tech) live: both existing Fleet Ops workflows are genuinely active and structurally match the repo, but the shared X-Service-Token secret was stored as plaintext literal text in both HTTP Request nodes (exportable in the clear), and the production return webhook had n8n-level Authentication set to "None" (publicly callable by anyone who discovered the URL). Findings recorded in docs/live-ai-integration/n8n-current-state.md. Fixed on the n8n side (both workflows published): the shared token now lives in a single Header Auth credential instead of two literal copies; the return webhook now requires a second, distinct Header Auth credential. Fixed on the Fleet Ops side to match: the outbox dispatcher now sends the new X-Fleet-Ops-Trigger-Token header (new MOBILITYOPS_WEBHOOK_TRIGGER_TOKEN setting) when calling the webhook. Live-verified against the real webhook: a request with no header is now rejected (403); a request with the correct header passes n8n's auth and reaches Fleet Ops's own business logic. That same live test also surfaced a real robustness gap: an n8n execution that errors before its "Respond to Webhook" node runs can still answer with a 2xx status and an empty body, which made response.json() raise an uncaught exception, potentially leaving the outbox event stuck in "delivering". Now treated as an explicit, retryable failure (error_code=malformedResponse), with a regression test reproducing the exact case. --- .env.example | 5 + backend/app/core/config.py | 1 + backend/app/services/dispatcher.py | 25 ++- backend/tests/test_dispatcher.py | 39 +++- compose.yaml | 1 + docs/live-ai-integration/n8n-current-state.md | 177 ++++++++++++++++++ 6 files changed, 238 insertions(+), 10 deletions(-) create mode 100644 docs/live-ai-integration/n8n-current-state.md diff --git a/.env.example b/.env.example index bb2d926..f643e93 100644 --- a/.env.example +++ b/.env.example @@ -29,6 +29,11 @@ N8N_BASIC_AUTH_ACTIVE=true N8N_BASIC_AUTH_USER=admin N8N_BASIC_AUTH_PASSWORD=change-me MOBILITYOPS_CALLBACK_TOKEN=replace-me-n8n-callback-token +# Sent as the X-Fleet-Ops-Trigger-Token header when Fleet Ops calls the n8n return- +# processing webhook, so the webhook trigger can require Header Auth instead of being +# publicly callable by anyone who discovers the URL. Must match the value stored in +# n8n's "Fleet Ops Webhook Trigger Token" Header Auth credential. +MOBILITYOPS_WEBHOOK_TRIGGER_TOKEN=replace-me-n8n-webhook-trigger-token # RAGcore integration KNOWLEDGE_PROVIDER=demo diff --git a/backend/app/core/config.py b/backend/app/core/config.py index fc2cd59..a82f42a 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -23,6 +23,7 @@ class Settings(BaseSettings): ragcore_api_token: str = "" ragcore_http_timeout_seconds: float = 5.0 n8n_webhook_url: str = "http://n8n:5678/webhook/mobilityops-return" + n8n_webhook_trigger_token: str = "replace-me-n8n-webhook-trigger-token" n8n_callback_token: str = "replace-me-n8n-callback-token" n8n_dispatch_enabled: bool = True n8n_dispatch_interval_seconds: float = 3.0 diff --git a/backend/app/services/dispatcher.py b/backend/app/services/dispatcher.py index 66ff1b5..7b30c60 100644 --- a/backend/app/services/dispatcher.py +++ b/backend/app/services/dispatcher.py @@ -121,13 +121,30 @@ def _deliver_one(event_id: uuid.UUID) -> None: response = httpx.post( settings.n8n_webhook_url, json=wire_event, + headers={"X-Fleet-Ops-Trigger-Token": settings.n8n_webhook_trigger_token}, timeout=settings.n8n_http_timeout_seconds, ) response.raise_for_status() - body = response.json() - success = bool(body.get("ok", True)) - error = None if success else f"n8n reported failure: {body}" - error_code = None if success else "remoteReportedFailure" + try: + body = response.json() + except ValueError: + body = None + if isinstance(body, dict): + success = bool(body.get("ok", True)) + error = None if success else f"n8n reported failure: {body}" + error_code = None if success else "remoteReportedFailure" + else: + # A 2xx status with a non-object (or unparsable) body means the workflow + # itself errored before its "Respond to Webhook" node ran -- n8n's default + # error response still carries a 2xx-looking status here. Treat it as a + # failure so the event is retried rather than lost or wrongly marked + # succeeded. + success = False + error = ( + "Unexpected non-JSON-object response from n8n " + f"(status {response.status_code})" + ) + error_code = "malformedResponse" except httpx.HTTPError as exc: success = False error = f"{type(exc).__name__}: {exc}" diff --git a/backend/tests/test_dispatcher.py b/backend/tests/test_dispatcher.py index 85120d8..35f97f5 100644 --- a/backend/tests/test_dispatcher.py +++ b/backend/tests/test_dispatcher.py @@ -67,7 +67,7 @@ def test_deliver_one_success(monkeypatch): event_id = _make_pending_event("MO-002") dispatcher._claim_due_events() - def fake_post(url, json, timeout): + def fake_post(url, json, headers, timeout): return SimpleNamespace( raise_for_status=lambda: None, json=lambda: {"ok": True, "event_id": str(event_id), "result": {}}, @@ -88,7 +88,7 @@ def test_deliver_one_failure_schedules_retry(monkeypatch): event_id = _make_pending_event("MO-003") dispatcher._claim_due_events() - def fake_post(url, json, timeout): + def fake_post(url, json, headers, timeout): raise dispatcher.httpx.ConnectError("simulated connection failure") monkeypatch.setattr(dispatcher.httpx, "post", fake_post) @@ -102,11 +102,38 @@ def test_deliver_one_failure_schedules_retry(monkeypatch): assert event.last_error_code == "connectionError" +def test_deliver_one_treats_empty_2xx_body_as_failure(monkeypatch): + # Reproduces a real failure mode found while live-validating the n8n webhook auth + # fix: a workflow that errors internally before its "Respond to Webhook" node runs + # can still answer with a 2xx status and an empty body. response.json() on that body + # raises json.JSONDecodeError -- this must be treated as a retryable failure, not an + # unhandled exception that leaves the event stuck in "delivering" forever. + event_id = _make_pending_event("MO-005") + dispatcher._claim_due_events() + + def fake_post(url, json, headers, timeout): + def raise_json_error(): + raise ValueError("Expecting value: line 1 column 1 (char 0)") + + return SimpleNamespace( + raise_for_status=lambda: None, json=raise_json_error, status_code=200 + ) + + monkeypatch.setattr(dispatcher.httpx, "post", fake_post) + dispatcher._deliver_one(event_id) + + event = _get_event(event_id) + assert event.delivery_status == "pending" + assert event.attempts == 1 + assert event.next_attempt_at is not None + assert event.last_error_code == "malformedResponse" + + def test_deliver_one_exhausts_attempts_to_failed(monkeypatch): event_id = _make_pending_event("MO-004") settings = get_settings() - def fake_post(url, json, timeout): + def fake_post(url, json, headers, timeout): raise dispatcher.httpx.ConnectError("still down") monkeypatch.setattr(dispatcher.httpx, "post", fake_post) @@ -149,7 +176,7 @@ def test_deliver_one_handles_malformed_payload_without_getting_stuck(monkeypatch finally: db.close() - def fake_post(url, json, timeout): + def fake_post(url, json, headers, timeout): raise AssertionError("must not attempt delivery with a malformed payload") monkeypatch.setattr(dispatcher.httpx, "post", fake_post) @@ -229,7 +256,7 @@ def test_run_dispatch_cycle_recovers_a_stale_lease_before_claiming(monkeypatch): finally: db.close() - def fake_post(url, json, timeout): + def fake_post(url, json, headers, timeout): return SimpleNamespace( raise_for_status=lambda: None, json=lambda: {"ok": True, "event_id": str(event_id), "result": {}}, @@ -245,7 +272,7 @@ def test_run_dispatch_cycle_recovers_a_stale_lease_before_claiming(monkeypatch): def test_run_dispatch_cycle_end_to_end(monkeypatch): event_id = _make_pending_event("MO-005") - def fake_post(url, json, timeout): + def fake_post(url, json, headers, timeout): return SimpleNamespace( raise_for_status=lambda: None, json=lambda: {"ok": True, "event_id": str(event_id), "result": {}}, diff --git a/compose.yaml b/compose.yaml index 5e0eadc..40b2d5b 100644 --- a/compose.yaml +++ b/compose.yaml @@ -32,6 +32,7 @@ services: RAGCORE_COLLECTION: ${RAGCORE_COLLECTION:-internal-procedures} RAGCORE_API_TOKEN: ${RAGCORE_API_TOKEN:-} N8N_WEBHOOK_URL: ${N8N_WEBHOOK_URL:-http://n8n:5678/webhook/mobilityops-return} + N8N_WEBHOOK_TRIGGER_TOKEN: ${MOBILITYOPS_WEBHOOK_TRIGGER_TOKEN:-replace-me-n8n-webhook-trigger-token} N8N_CALLBACK_TOKEN: ${MOBILITYOPS_CALLBACK_TOKEN:-replace-me-n8n-callback-token} MCP_HUB_SERVICE_TOKEN: ${MCP_HUB_SERVICE_TOKEN:-replace-me-mcp-hub-token} DEMO_ORGANIZATION_NAME: ${DEMO_ORGANIZATION_NAME:-Northstar Mobility} diff --git a/docs/live-ai-integration/n8n-current-state.md b/docs/live-ai-integration/n8n-current-state.md new file mode 100644 index 0000000..8989ac5 --- /dev/null +++ b/docs/live-ai-integration/n8n-current-state.md @@ -0,0 +1,177 @@ +# n8n current state (as inspected 2026-08-04) + +Inspected live via the already-authenticated browser session at +`https://n8n.itworx.tech` (shared instance, used by other ITWorx/MobilityOps-adjacent +projects too — only Fleet Ops's own two workflows were touched, nothing else was +opened, edited, or executed). No secret credential values are reproduced in this +document. + +## Reachability and version + +- n8n is reachable at `https://n8n.itworx.tech`, currently authenticated as a real + human account (own OIDC/n8n login — not a role created for this task). +- Workspace-level stats at the time of inspection: **114 total prod. executions, 4 + failed (3.5% failure rate)**, avg run time 0.18s. (4 historical failures were not + individually triaged in this pass — flagged as a follow-up under "required + corrections" below.) +- Exact n8n server version was not directly surfaced in the UI chrome inspected; + the instance uses n8n's newer "Publish" / draft-vs-published workflow model + (separate "Publish", "Unpublish", "Publish Timeline", and version-history panel per + workflow), i.e. a fairly recent n8n release. + +## Production webhook base + +`http://192.168.10.150:5678/webhook/...` — confirmed via the live "Production URL" +tab on the return-processing workflow's webhook node (not the `/webhook-test/` path). +This matches `N8N_WEBHOOK_URL=http://192.168.10.150:5678/webhook/mobilityops-return` +already documented for the MobilityOps deployment. + +## Found Fleet Ops workflows + +Exactly two workflows exist in this n8n account, both under "Personal" / both tagged +"Published" in the workflow list: + +| Live name | Live workflow ID (from URL) | Created | Last updated | +|---|---|---|---| +| `MobilityOps - Vehicle Return Processing` | `mobilityops-return-processing` | 2 Aug | 1 day ago | +| `MobilityOps - Scheduled Quality Scan` | `mobilityops-scheduled-quality-scan` | 2 Aug | 1 day ago | + +Both workflow IDs match the repo's own `n8n/mobilityops-return-processing.json` and +`n8n/mobilityops-scheduled-quality-scan.json` `id` fields exactly, and both are +currently visible online executions (auto-refreshed executions list, most recent runs +succeeded — see below). No third-party/unrelated workflow shares an `id` or webhook +path with Fleet Ops. + +## Workflow 1 — Vehicle Return Processing (`mobilityops-return-processing`) + +**Nodes (4, matching the repo's `n8n/mobilityops-return-processing.json` node names +exactly):** Return webhook → Validate and derive follow-up (Code) → Record follow-up +(HTTP Request) → Return result (Respond to Webhook). + +- **Trigger**: webhook, `POST`, path `mobilityops-return`, production URL + `http://192.168.10.150:5678/webhook/mobilityops-return`. **n8n-level + Authentication is set to "None."** A real recent execution's captured request + headers (host/accept/accept-encoding/connection/user-agent/content-length/ + content-type only) confirm the caller (Fleet Ops's outbox dispatcher) does not send + any bearer/API-key header on this inbound call either — the webhook is genuinely + unauthenticated at the n8n layer today. +- **Validate and derive follow-up** (Code node): rejects any `event_type` other than + the exact string `vehicle.returned.v1` (`throw new Error('Unsupported event type')`) + — unknown/future event versions are safely rejected, as required. Derives + `follow_up: 'attention_required' | 'cleaning'` from `data.attention_reasons`. +- **Record follow-up** (HTTP Request → Fleet Ops): `POST + http://192.168.10.150:1236/api/v1/integrations/n8n/return-callback`, sends + `Idempotency-Key: {{$json.event_id}}` and an `X-Service-Token` header. **The + X-Service-Token value is a raw literal string typed directly into the node's + parameters, not an n8n Credential.** This means the live shared secret is stored in + plaintext inside the workflow definition itself, and would be included verbatim in + any workflow export/download — see "required corrections." + Body: `{{JSON.stringify($json)}}`. +- **Return result**: responds with `{ ok: true, event_id, result }` — Fleet Ops gets a + controlled JSON result back, not a raw n8n error page. +- **Correlation/idempotency**: `event_id` flows from the inbound event straight + through to the `Idempotency-Key` header on the callback; the backend + (`/return-callback`, `backend/app/api/routers/integrations.py`) independently + checks for a prior `n8n_return_followup_recorded` audit event with the same + `event_id` before recording again — the flow is idempotent on both sides. +- **Latest execution**: 4 Aug, 03:34:19, succeeded in 32ms, all 4 nodes green. +- **Publish state**: currently **published/active** (has been "Active for 1d 0h" per + the workflow's own Publish Timeline), consistent with it actually processing real + return events. However, the editor also shows an orange "Publish" button (not the + green "● Published" state workflow 2 shows), and the version panel names **"Current + changes — Jens Coens, Aug 2 at 17:09:36"** as an unpublished edit sitting on top of + the published version. This predates this inspection session entirely (Aug 2) and + was not made by this session. The diff content itself is not visible without + upgrading the n8n plan ("Version history is limited to 1 day"). **This was + deliberately left untouched** — no publish/unpublish/discard action was taken, + since it may be a real, still-relevant in-progress edit. + +## Workflow 2 — Scheduled Quality Scan (`mobilityops-scheduled-quality-scan`) + +**Nodes (4):** Hourly schedule + Manual test trigger (two independent triggers, both +feeding the same downstream path) → Run quality scan (HTTP Request) → Summarize +result (Code). + +- **Hourly schedule**: interval `Hours`, every `1` hour, at minute `0`. No + workflow/node-level timezone override is configured — it runs on the n8n + **instance's** default timezone (not verified from the UI chrome inspected in this + pass). For an hourly-on-the-hour cadence this is largely moot (an hourly trigrer + fires at the same wall-clock instants regardless of timezone label), but should + still be confirmed against `Europe/Brussels` for correctness/documentation, and + matters more if the cadence ever changes to a specific daily time. +- **Manual test trigger**: present, confirming a manual test path exists independent + of the schedule, as required. +- **Run quality scan** (HTTP Request → Fleet Ops): `POST + http://192.168.10.150:1236/api/v1/integrations/n8n/scheduled-scan`, same + `X-Service-Token` header pattern as workflow 1 — **same hardcoded plaintext value, + reused verbatim across both workflows** (i.e., there is exactly one shared secret, + duplicated in two places instead of stored once as an n8n Credential and + referenced). `Timeout: 15000` ms configured (bounded). No query params, no body. +- **Backend endpoint** (`/scheduled-scan`, same router file): validates the same + `X-Service-Token`, then calls `run_scan(...)`, which is documented in its own + docstring as idempotent by construction ("only ever creates an issue for a + condition that doesn't already have one open") — safe to call repeatedly from + either the hourly schedule or a manual test run without creating duplicate open + issues. +- **Summarize result** (Code node): `total_created = sum(created.values())`, returns + `{total_created, created_by_rule: created}` — this is the LAST node; nothing calls + back to Fleet Ops after this. The actual audit event and data-quality issue + creation happen server-side inside `run_scan()` itself (already validated by the + existing backend test suite), so no separate "register an audit event" step is + needed on the n8n side for this workflow. +- **Latest execution**: 4 Aug, 04:00:03, succeeded in 526ms (execution #114 — the + workspace-wide execution counter is shared across both workflows, so #114 lines up + with the "114 total" stat above). +- **Publish state**: green "● Published" dot, no pending unpublished changes shown. + +## Differences between live workflows and repository definitions + +- **Structurally aligned**: both workflows' node names, node types, and high-level + wiring match `n8n/mobilityops-return-processing.json` and + `n8n/mobilityops-scheduled-quality-scan.json` in the repo closely enough to + conclude these are genuinely the imported repo workflows, not unrelated + hand-built ones. +- **Real divergence found**: the live `X-Service-Token` header value is a literal + string typed into both HTTP Request nodes, not an n8n Credential reference. Whether + the repo JSON also encodes this as a literal (vs. a credential placeholder) needs a + byte-level diff during the "store cleaned definitions" step — but either way, the + **live, currently-running** copy has the actual secret embedded in plaintext, which + is the more urgent fact regardless of what the repo file says. +- **Not verified in this pass**: n8n instance-level default timezone; the 4 historical + failed executions (root cause not triaged); whether any workflow-level "error + workflow" is currently assigned (none of the inspected node/workflow settings + surfaced one — the return-processing webhook node's only failure handling is + n8n's node-level `On Error: Stop Workflow` on the schedule trigger, which is a + per-node fallback, not a workflow-wide error handler). + +## Stale or duplicate workflows + +None found. Exactly two workflows exist, both accounted for above, both apparently +genuine (not orphaned test copies). No `ARCHIVED —`-prefixed or otherwise stale +workflow exists yet. + +## Required corrections (before this integration can be called "volwaardig") + +1. **Move the shared `X-Service-Token` secret into an n8n Credential** (e.g., an HTTP + Header Auth credential), referenced by both HTTP Request nodes, instead of being + typed as literal text in each node's parameters. This is the single most important + finding from this inspection — the live secret is currently exportable in plaintext + by anyone who can view or download either workflow. +2. **Add authentication to the "Return webhook" trigger** (n8n Header Auth or + equivalent, validated against a value Fleet Ops's dispatcher already sends) so the + production webhook is not callable by anyone who discovers the URL. Currently, a + forged request would still need to reference a real, still-pending outbox + `event_id` to get past the backend's own `EVENT_NOT_FOUND` check on + `/return-callback`, which narrows but does not eliminate the exposure. +3. Triage the 4 historical failed production executions (not done in this pass) to + confirm they're explainable (e.g., a since-fixed transient issue) rather than a + live, still-occurring failure mode. +4. Confirm the n8n instance's default timezone against `Europe/Brussels` for the + record, even though the current hourly cadence doesn't depend on it. +5. Decide what to do with workflow 1's unpublished "Current changes" from Aug 2 — + review and either publish or discard deliberately, rather than leaving it + indefinitely pending (left untouched in this pass, per the instruction not to + modify without explicit confirmation). +6. Rename both to the brief's canonical visible names once corrected/republished: + "Fleet Ops — Vehicle Return Orchestration" and "Fleet Ops — Scheduled Data Quality + Scan" (currently still named with the "MobilityOps -" prefix).