Slack and Discord OAuth state consumption is non-atomic and fail-open

Summary

Slack and Discord OAuth callbacks consume their CSRF state values with a non-atomic Redis GET followed by DEL. The delete helper suppresses all failures, and the callback proceeds regardless. Concurrent callbacks can therefore both validate one state, and a transient delete failure leaves the state replayable for its remaining 10-minute TTL.

Reviewed at commit 396efb905fadda74c4ae77080a1e72658c37aa0e.

Evidence

Slack:

• api/slack/oauth/start.ts:65 stores wm:slack:oauth:${state} for 600 seconds.
• api/slack/oauth/callback.ts:141-144 performs upstashGet(stateKey), then upstashDel(stateKey).
• api/slack/oauth/callback.ts:59-64 catches and ignores every delete failure.

Discord has the same pattern:

• api/discord/oauth/start.ts:65
• api/discord/oauth/callback.ts:120-123
• api/discord/oauth/callback.ts:44-49

There is no lock, GETDEL, Lua transaction, or checked delete result.

Security impact

OAuth state is intended to be single-use. If a state value is exposed, two callback requests using different provider authorization codes can race past the read before either delete completes; both are then associated with the userId stored under that state. On delete failure, the same condition persists until TTL expiry. Because channel storage happens after the provider exchange, a later callback can overwrite the Slack/Discord channel connected to the victim account.

The issue requires possession of an unexpired state and is therefore best treated as OAuth hardening rather than an unauthenticated standalone takeover, but the current "consume - prevents replay" comment is not true under concurrency or Redis failure.

Recommended fix

1. Consume state atomically with Redis GETDEL, or a Lua script that returns the value while deleting it.
2. Fail closed when state consumption cannot be confirmed; do not continue on Redis delete failure.
3. Add concurrency tests where two callbacks race on one state and exactly one may reach token exchange.
4. Add a Redis-failure test proving a callback does not proceed when atomic consumption is unavailable.
5. Consider binding state to provider and a browser-side nonce/PKCE verifier for additional defense in depth.

Acceptance criteria

• Each OAuth state can authorize at most one callback.
• State-store outages fail closed before provider token exchange or channel mutation.
• Slack and Discord share one tested atomic state-consumption helper.