fix(oauth): a provider connect must send the credential the remote authenticates

[rimusz]

Connecting any OAuth provider fails at the last step with a raw database error in a user-facing dialog, after the user has already authorised the third-party app:

Connection Error
Failed to connect to Github: SQLITE_CONSTRAINT: NOT NULL constraint failed: oauth_tokens.user_id

The handshake with the provider succeeds. The failure is on the remote's write: it stores the token with user_id = NULL against a user_id TEXT NOT NULL column.

The remote does not read the bearer token on these routes. Against api.agnt.gg, POST /auth/callback returns that identical error with a valid JWT, with a deliberately malformed JWT, and with no Authorization header at all — three requests differing only in the credential, one response. Its CORS reply says what it does read:

access-control-allow-origin: http://localhost:3333      (echoed, not *)
access-control-allow-credentials: true

A server only echoes a specific origin and sets allow-credentials: true when it expects cookie credentials. Sign-in already talks to that host that way — store/auth/userAuth.js passes withCredentials: true on every magic-link call — but none of the OAuth call sites did. The one request that has to be attributed to a user was the one carrying nothing the remote reads.

What this changes

credentials: 'include' (fetch) / withCredentials: true (axios) on every call to /auth/connect/ and /auth/callback.

Ten call sites, not three. connectOAuthApp and the callback exchange are copy-pasted across six screens:

Call site	Route
views/_components/utility/OAuthCallback.vue:67	callback — the one that produced the error above
services/providerAuthService.js:84	callback (Electron postMessage path)
…/screens/Connectors/Connectors.vue:2083	callback
…/_OauthManager/OauthManager.vue:360	callback
composables/useProviderConnection.js:331	connect
…/screens/Connectors/Connectors.vue:1209	connect
…/_OauthManager/OauthManager.vue:223	connect
…/ChatPanel/components/IntegrationHealth.vue:356	connect
components/OnboardingModal.vue:590	connect
…/Chat/components/ProviderSetup.vue:190	connect

Fixing a subset would be worse than fixing none: connecting would work or fail depending on which screen the user clicked Connect from. I found this the hard way — my first pass patched three, and inspecting the built bundle showed the Connectors chunk still shipping an unpatched call.

What is not verified

This is a hypothesis fix and should be reviewed as one. No Set-Cookie is observable from api.agnt.gg on any endpoint reachable without completing a fresh sign-in, so the existence and SameSite attributes of the session cookie are inferred from the CORS configuration and the sign-in code, not observed. If that cookie is SameSite=Lax it will not be sent cross-site and this change is inert.

A maintainer can settle this instantly — does /auth/callback resolve the user from a session cookie? If no, close this; the real bug is entirely #75.

It cannot regress anything either way: it adds a credential to requests to the app's own auth host and changes no control flow.

The server-side defect stands regardless

Filed as #75. /auth/callback accepts a request it cannot attribute to any user, attempts the insert anyway, and returns the storage error rather than refusing with 401. Reproducible with one curl and no account. That is why this took a database error to diagnose instead of an auth one — same shape as #71, where an infrastructure failure was reported as a domain value.

Tests

frontend/src/services/providerAuthService.spec.js already pinned the exact request shape for the callback exchange, so it correctly failed on this change and is updated. The cookie is additionally asserted on its own, so a refactor of the options object cannot drop it without a failing test that names what broke. Removing withCredentials from the service fails both (verified).

That covers 1 of the 10 call sites. The second commit adds frontend/src/__guards__/oauthCredentialContract.spec.js, following apiAuthContract.spec.js, whose reason for existing is the same shape: a rule no single module owns, copy-pasted until someone forgets it. It derives the call sites from the source, so a seventh copy cannot be added without either carrying the credential or failing CI.

Run against the tree without the fix, the guard names all ten — an independent check on the claim that ten is the whole set, since it scans rather than reusing the grep that found them:

AssertionError: these calls reach the remote auth host without the session credential …
+   "components/OnboardingModal.vue:590 -> /auth/connect/",
+   "composables/useProviderConnection.js:330 -> /auth/connect/",
+   "services/providerAuthService.js:84 -> /auth/callback",
…10 total

Three of its self-tests target failure modes of the guard rather than of the code: call text is paren-matched rather than windowed (Connectors.vue holds two calls, and a window lets a credentialed neighbour vouch for an uncredentialed one — a false negative, the only direction that matters); the backward scan counts depth because the URLs embed encodeURIComponent(...); and a minimum call-site count is asserted so that renaming API_CONFIG.REMOTE_URL cannot make the contract pass vacuously.

Full frontend suite: 224 files / 3925 tests, all passing.

Review round

Copilot caught a real hole in the guard: the flag was matched against the raw call text, so a commented-out // credentials: 'include', read as compliant. Confirmed by reproducing it, then fixed in 741ffb81. The obvious fix is unsafe in the other direction — a //.*$ sweep turns 'github:http://localhost:3333' into 'github:http:, corrupting the source and deleting any flag after a URL on the same line — so the strip is string-aware, and both directions now have self-tests.

Relationship to other open PRs

Independent of #70–#74 — no shared files (those are backend workflow/plugin changes; this is frontend only). Branched from origin/main at 2bffa7e2.

[Copilot]

🟡 Changes recommended

The new contract test can yield false negatives because it treats comment text as satisfying the credential flag check, weakening the intended CI protection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes OAuth provider connection failures by ensuring all frontend calls to the remote auth host (/auth/connect/* and /auth/callback) include cookie credentials (credentials: 'include' / withCredentials: true) so the remote can attribute the request to the signed-in user session.

Changes:

• Add credentials: 'include' to all fetch() call sites hitting the remote OAuth connect/callback routes.
• Add withCredentials: true to the axios-based callback exchange and update its unit tests accordingly.
• Add a guard test (oauthCredentialContract.spec.js) that scans the source tree to prevent future regressions where a new call site forgets to include credentials.

File summaries

File	Description
frontend/src/views/Terminal/RightPanel/types/ChatPanel/components/IntegrationHealth.vue	Include cookie credentials on remote /auth/connect/* fetch.
frontend/src/views/Terminal/CenterPanel/screens/Settings/components/_OauthManager/OauthManager.vue	Include cookie credentials on remote /auth/connect/* and /auth/callback fetches.
frontend/src/views/Terminal/CenterPanel/screens/Connectors/Connectors.vue	Include cookie credentials on remote /auth/connect/* and /auth/callback fetches.
frontend/src/views/Terminal/CenterPanel/screens/Chat/components/ProviderSetup.vue	Include cookie credentials on remote /auth/connect/* fetch.
frontend/src/views/_components/utility/OAuthCallback.vue	Include cookie credentials on remote /auth/callback fetch.
frontend/src/services/providerAuthService.js	Add withCredentials: true for axios POST to remote /auth/callback.
frontend/src/services/providerAuthService.spec.js	Update assertions and add coverage to ensure withCredentials is preserved.
frontend/src/composables/useProviderConnection.js	Include cookie credentials on remote /auth/connect/* fetch.
frontend/src/components/OnboardingModal.vue	Include cookie credentials on remote /auth/connect/* fetch.
frontend/src/guards/oauthCredentialContract.spec.js	Add source-scanning contract test enforcing credentials on remote OAuth routes.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[rimusz]

Confirmed and fixed in 741ffb8 — thank you, this was a real hole.

I reproduced it before changing anything. The detector returned sendsCredentials = true for:

await fetch(`${API_CONFIG.REMOTE_URL}/auth/connect/${id}`, {
  // credentials: 'include',   <- disabled while debugging
  headers: { Authorization: 'x' },
});

A guard whose whole purpose is catching an omission must not be satisfied by a comment describing the thing it checks for.

One wrinkle worth recording: the obvious fix is unsafe. A //.*$ sweep turns

'github:http://localhost:3333'   into   'github:http:

which corrupts the source being read and would delete any flag sitting after a URL on the same line — a false positive, the opposite error. Since every one of these call sites is built around a URL, that would have bitten quickly. So stripComments is string/template-aware and skips over quoted regions.

Three self-tests now cover both directions: line-commented and block-commented flags must fail the contract, and a URL inside a string must still read as compliant. Verified by mutation — reducing stripComments to the identity function fails exactly the two comment tests, and the contract still names all ten call sites when run against the tree without the fix.

Known limit, documented in the source: a regex literal containing // would confuse it. None of these call sites contain one, and that would surface as a loud failure rather than a silent pass.

[agnt-gg]

Thanks for this one — and for flagging your own uncertainty in "What is not verified". That note is the reason this got checked properly rather than merged on plausibility.

The premise doesn't hold, so I'm closing this. The remote does read the bearer on these routes:

// api.agnt.gg/src/routes/AuthRoutes.js
router.post('/callback', authenticateToken, rateLimiter, AuthController.handleCallback);

// api.agnt.gg/src/routes/Middleware.js — authenticateToken
const token = req.headers['authorization'].split(' ')[1];
const decoded = jwt.verify(token, process.env.JWT_SECRET);
req.user = { id: decoded.id, ... };

req.session appears nowhere in that path. Confirmed live against production:

request	result
valid bearer	200 — returns the caller's connected providers
malformed bearer	200 []
no header at all	200 []

The access-control-allow-credentials: true you inferred from is global CORS config, not evidence that cookies carry the session here.

I also read all ten call sites this PR touches — every one already sends Authorization: Bearer ${token}. So adding withCredentials/credentials: 'include' attaches a cookie to requests that already carry the exact credential the server authenticates with. It is inert rather than wrong, but it would leave behind a misleading signal that these routes are cookie-authenticated.

Your diagnosis in #75 was right, and the real fix has shipped server-side. The cause was that authenticateToken identifies but never refuses: on a missing, invalid or unprovable token it sets { isAuthenticated: false } and calls next(). req.user.id then arrived at the INSERT as undefined, and SQLite became the component enforcing authentication — which is why an anonymous request got a schema disclosure back.

Fixed by putting the hard guard on the four routes that read caller identity, plus proper error mapping (401/404/502/500 with a stable reason, never error.message), and a single-use nonce in state to close the OAuth-CSRF gap. An audit that fails the build when a handler reads req.user.id on a route that cannot refuse found 15 more routes in the same shape; all are now guarded.

Genuinely appreciate the work in this series — thank you.