Skip to content

Add source column to AuditLog with request context propagation#7825

Merged
cstns merged 6 commits into
mainfrom
feat/7562-audit-source-column
Jul 17, 2026
Merged

Add source column to AuditLog with request context propagation#7825
cstns merged 6 commits into
mainfrom
feat/7562-audit-source-column

Conversation

@cstns

@cstns cstns commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Adds infrastructure so audit log entries can record how an action was triggered, not just who triggered it. A new source column on the AuditLog table distinguishes cookie sessions (null), direct PAT API calls (api), first party expert agent calls (mcp:expert), and eventually third party MCP agents (mcp).

  • New source column on AuditLogs (nullable, null = legacy/cookie session)
  • Generic in-memory nonce store (app.nonceStore) for process-local trust boundaries. Nonces are single-use and short-lived (30s TTL). Includes a createSourceNonce() factory that validates required fields and auto-generates a correlationId when not provided.
  • resolveSourceContext in verifySession consumes nonces and populates sourceContext in @fastify/request-context. Falls back to source: 'api' with tokenId for plain PAT calls, null for cookie sessions.
  • Audit log controllers read from request context and write the source column. When source context includes a correlationId (MCP calls), a sourceContext object is merged into the audit body with toolName, correlationId, and tokenId.
  • Formatter and view updates to include source in API output.

Anti-spoofing: external requests can't set a fake source because only in-process code can mint nonces via app.nonceStore. No valid nonce means no MCP source, regardless of what headers a caller sends.

Related Issue(s)

closes #7562

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@cstns cstns self-assigned this Jul 15, 2026
@cstns cstns added the area:migration Involves a database migration label Jul 15, 2026
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.39130% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.17%. Comparing base (844a704) to head (7dfc783).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
forge/lib/nonceStore.js 87.50% 5 Missing ⚠️
...orge/db/migrations/20260714-01-add-audit-source.js 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7825      +/-   ##
==========================================
+ Coverage   75.10%   75.17%   +0.06%     
==========================================
  Files         430      432       +2     
  Lines       22846    22933      +87     
  Branches     6055     6076      +21     
==========================================
+ Hits        17159    17239      +80     
- Misses       5687     5694       +7     
Flag Coverage Δ
backend 75.17% <92.39%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cstns
cstns requested review from andypalmi and knolleary July 15, 2026 11:23
@cstns
cstns marked this pull request as draft July 15, 2026 11:23
@cstns
cstns requested a review from hardillb July 15, 2026 11:24
@andypalmi

Copy link
Copy Markdown
Contributor

Test coverage gap: the nonce trust boundary is untested

resolveSourceContext and the x-ff-source-nonce path in forge/routes/auth/index.js have no test coverage. The controller tests in AuditLog_spec.js monkeypatch requestContext.get, which bypasses the actual source-resolution logic in auth.

The most security-relevant case is never exercised: a request with a bogus or expired nonce must not receive an MCP source. Suggested auth-level cases:

  • bogus/expired nonce -> source stays null
  • valid minted nonce -> sourceContext populated from the nonce metadata
  • PAT session without a nonce -> source api with tokenId

Since this is the anti-spoofing boundary, it is worth covering here rather than deferring to #7563.

@andypalmi

Copy link
Copy Markdown
Contributor

Follow-up: reconcile the downstream issues with the nonce approach

The PR description documents the nonce mechanism (x-ff-source-nonce plus app.nonceStore) and its anti-spoofing rationale well. Flagging only that the downstream artifacts still describe the earlier header scheme (X-FF-Audit-Source / X-FF-Audit-Token) and will need updating so the story stays consistent:

Not a change to this PR, just a heads up for the follow-up work.

@andypalmi

Copy link
Copy Markdown
Contributor

createSourceNonce mutates the caller's metadata object

In forge/lib/nonceStore.js, createSourceNonce assigns metadata.correlationId back onto the passed-in object when one is not provided. A caller that reuses or inspects that object afterwards will find a correlationId it did not set. Cloning first avoids the side effect:

const meta = { ...metadata }
if (!meta.correlationId) {
    meta.correlationId = crypto.randomUUID()
}
return this.create(meta, ttl)

@andypalmi

Copy link
Copy Markdown
Contributor

Minor: body.sourceContext could collide with an existing body key

applySourceContext in forge/db/controllers/AuditLog.js, and the passthrough in forge/auditLog/formatters.js, merge sourceContext into the audit body. If an audit event ever carried its own sourceContext key it would be overwritten. This is practically impossible for current audit bodies, so noting only for awareness. No change needed unless you want a defensive guard.

@andypalmi

Copy link
Copy Markdown
Contributor

What is solid

  • The nonce approach is a stronger anti-spoofing model than trusting headers: external callers cannot mint a nonce, so no valid nonce means no MCP source regardless of what headers are sent.
  • The nonce is checked first on both the cookie and PAT auth paths, is single-use (deleted before the expiry check), TTL bound, swept periodically, and the sweep timer is unref'd.
  • Cross app.inject() propagation is correct: the nonce is re-resolved inside the inner request, so the source context lands in that request's context store and the audit controller reads it back. This mirrors the existing pat / isPAT mechanism from Story 0: Add PAT (Personal Acces Tokens) scopes #7411.
  • Reading source context is safe outside a request: requestContext.get returns undefined when no store is active, so audit writes from background tasks stay source: null rather than throwing.
  • The migration is nullable and backwards compatible, and is correctly the latest migration by date.
  • Good unit coverage for the nonce store, formatters, and controller source handling.

@cstns
cstns marked this pull request as ready for review July 16, 2026 12:15

@andypalmi andypalmi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for the quick turnaround on the earlier notes. The nonce clone fix is in, and the new resolveSourceContext integration test covers exactly the anti-spoofing cases that matter: bogus nonce, expired nonce, valid nonce, PAT fallback, and single-use reuse, all checked end to end against the audit source column. That was the main thing I wanted to see.

The nonce approach reads as a solid anti-spoofing model, and the migration is nicely backwards compatible. Approving. Nothing blocking from me.

Comment thread forge/lib/nonceStore.js
@hardillb hardillb mentioned this pull request Jul 17, 2026
11 tasks
@hardillb

Copy link
Copy Markdown
Contributor

This will need types re-generated if merged after #7733

@cstns

cstns commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

will merge it now

@cstns
cstns merged commit 19e0c9a into main Jul 17, 2026
29 checks passed
@cstns
cstns deleted the feat/7562-audit-source-column branch July 17, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:migration Involves a database migration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3rd Party MCP - Audit Source Column + Request Context Propagation

3 participants