Skip to content

Commit 6c0dbf0

Browse files
committed
chore: harden app security boundaries
1 parent 21d585f commit 6c0dbf0

77 files changed

Lines changed: 3493 additions & 1188 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/source-code-audit-2026-06-23.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ Do these early when they're obvious and isolated, but don't let them block highe
3333

3434
These are small enough to do before deeper refactors and they reduce risk while larger helpers are being designed.
3535

36-
- Analytics proxy header allowlist, GitHub webhook fail-closed secret, Cloudflare-first IP extraction, route response filename sanitizing, Discord raw-body size/signature-shape guard, MCP transport content-type/length guard and masked errors.
37-
- Public pagination and route-search caps, admin pagination/range caps, docs feedback content caps, showcase field/array caps, Shopify page-size/quantity/discount caps, UploadThing client preflight caps, image transform clamps, community-resource frontmatter schema.
38-
- URL/link hardening: Intent metadata URLs, Intent tarball source paths, markdown/source/search link classification, legacy redirect segment matching, local docs path containment.
39-
- OAuth/login quick fixes: return-to key mismatch, login modal stale callbacks, provider route param picklists, session revocation conflict retry, and session base64 fallback removal once covered.
36+
- [done 2026-06-24: `pnpm test:tsc`, `pnpm test:lint`, `pnpm test`] Analytics proxy header allowlist, GitHub webhook fail-closed secret, Cloudflare-first IP extraction, route response filename sanitizing, Discord raw-body size/signature-shape guard, MCP transport content-type/length guard and masked errors.
37+
- [done 2026-06-24: `pnpm test:tsc`, `pnpm test:lint`, `pnpm test`] Public pagination and route-search caps, admin pagination/range caps, docs feedback content caps, showcase field/array caps, Shopify page-size/quantity/discount caps, UploadThing client preflight caps, image transform clamps, community-resource frontmatter schema.
38+
- [done 2026-06-24: `pnpm test:tsc`, `pnpm test:lint`, `pnpm test`] URL/link hardening: Intent metadata URLs, Intent tarball source paths, markdown/source/search link classification, legacy redirect segment matching, local docs path containment.
39+
- [done 2026-06-24: `pnpm test:tsc`, `pnpm test:lint`, `pnpm test`] OAuth/login quick fixes: return-to key mismatch, login modal stale callbacks, provider route param picklists, session revocation conflict retry, and session base64 fallback removal once covered.
4040

4141
### 4. Small controllers that prevent repeat bugs
4242

@@ -89,6 +89,8 @@ Treat these as tracked initiatives, not normal cleanup PRs.
8989

9090
## Batch Log
9191

92+
- 2026-06-24: Landed the grouped security pass across public request boundaries, OAuth/login/session flows, builder/GitHub deploy APIs, outbound fetches, stats/Intent/docs/shop/showcase caps, UploadThing/image preflight, markdown/search link classification, docs redirects/path containment, and community-resource frontmatter validation.
93+
- 2026-06-24: Second pass simplified the security patch by sharing bounded body readers, npm package-name/page/date/chart schemas, and URL normalization; removed the local application-starter request guard while preserving same-origin/content-type/body-size enforcement.
9294
- 2026-06-24: Added shared response filename/content-disposition helpers, package route slug encoding helpers, and row-local id helpers; wired them into builder/docs downloads, Intent registry links, and moderation note inputs.
9395
- 2026-06-24: Hardened shared UI primitives: defaulted shop buttons and shared select triggers to non-submit buttons, gave pagination a unique page-size label/id pair plus non-submit controls, and made Tooltip merge trigger handlers/refs without `any`.
9496
- 2026-06-24: Added `test:unit` and wired it into `pnpm test` so existing TypeScript assertion tests run in the default validation path.

src/auth/cli-tickets.server.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
const TICKET_TTL_MS = 5 * 60 * 1000
12+
const MAX_TICKETS = 1_000
1213

1314
interface CliTicket {
1415
userId: string | null
@@ -38,18 +39,34 @@ const tickets: Map<string, CliTicket> = (globalScope[TICKETS_KEY] ??= new Map<
3839
if (!globalScope[TICKETS_INTERVAL_KEY]) {
3940
globalScope[TICKETS_INTERVAL_KEY] = setInterval(
4041
() => {
41-
const now = Date.now()
42-
for (const [id, ticket] of tickets) {
43-
if (ticket.expiresAt < now) {
44-
tickets.delete(id)
45-
}
46-
}
42+
cleanupExpiredTickets()
4743
},
4844
60 * 1000, // every minute
4945
)
5046
}
5147

48+
function cleanupExpiredTickets() {
49+
const now = Date.now()
50+
for (const [id, ticket] of tickets) {
51+
if (ticket.expiresAt < now) {
52+
tickets.delete(id)
53+
}
54+
}
55+
}
56+
57+
function trimTicketStore() {
58+
cleanupExpiredTickets()
59+
60+
while (tickets.size >= MAX_TICKETS) {
61+
const oldestId = tickets.keys().next().value
62+
if (!oldestId) break
63+
tickets.delete(oldestId)
64+
}
65+
}
66+
5267
export function createCliTicket(): string {
68+
trimTicketStore()
69+
5370
const id = crypto.randomUUID()
5471
tickets.set(id, {
5572
userId: null,

src/auth/client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const authClient = {
4040
const height = 600
4141
const left = window.screenX + (window.outerWidth - width) / 2
4242
const top = window.screenY + (window.outerHeight - height) / 2
43-
window.open(
43+
return window.open(
4444
`/auth/${provider}/start?popup=true`,
4545
'tanstack-oauth',
4646
`width=${width},height=${height},left=${left},top=${top},popup=yes`,
@@ -74,7 +74,7 @@ export function navigateToSignIn(
7474
window.location.href = url
7575
} else {
7676
const url = returnTo
77-
? `/login?returnTo=${encodeURIComponent(returnTo)}`
77+
? `/login?redirect=${encodeURIComponent(returnTo)}`
7878
: '/login'
7979
window.location.href = url
8080
}

src/auth/oauthClient.server.ts

Lines changed: 183 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
} from '~/db/schema'
77
import { eq, and, sql, lt } from 'drizzle-orm'
88
import { sha256Hex } from '~/utils/hash'
9+
import { envFunctions } from '~/utils/env.functions'
910

1011
// Token TTLs
1112
const AUTH_CODE_TTL_MS = 10 * 60 * 1000 // 10 minutes
@@ -15,6 +16,9 @@ const REFRESH_TOKEN_TTL_MS = 30 * 24 * 60 * 60 * 1000 // 30 days
1516
// Token prefixes (new generic prefixes)
1617
const ACCESS_TOKEN_PREFIX = 'oa_'
1718
const REFRESH_TOKEN_PREFIX = 'oar_'
19+
const REGISTERED_CLIENT_PREFIX = 'tsr_'
20+
const LAST_USED_WRITE_INTERVAL_MS = 5 * 60 * 1000
21+
const MAX_REGISTERED_CLIENT_PAYLOAD_LENGTH = 8192
1822

1923
// Legacy prefixes for backwards compatibility
2024
const LEGACY_ACCESS_TOKEN_PREFIX = 'mcp_'
@@ -33,6 +37,161 @@ export type OAuthValidationResult =
3337
status: number
3438
}
3539

40+
function isRecord(value: unknown): value is Record<string, unknown> {
41+
return typeof value === 'object' && value !== null && !Array.isArray(value)
42+
}
43+
44+
function isStringArray(value: unknown): value is Array<string> {
45+
return Array.isArray(value) && value.every((item) => typeof item === 'string')
46+
}
47+
48+
function normalizeRedirectUri(uri: string): string | null {
49+
try {
50+
return new URL(uri).toString()
51+
} catch {
52+
return null
53+
}
54+
}
55+
56+
function bytesToBase64Url(bytes: Uint8Array) {
57+
let binary = ''
58+
for (const byte of bytes) {
59+
binary += String.fromCharCode(byte)
60+
}
61+
62+
return btoa(binary).replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '')
63+
}
64+
65+
function stringToBase64Url(value: string) {
66+
return bytesToBase64Url(new TextEncoder().encode(value))
67+
}
68+
69+
function base64UrlToString(value: string): string | null {
70+
try {
71+
const padded = value
72+
.replace(/-/g, '+')
73+
.replace(/_/g, '/')
74+
.padEnd(Math.ceil(value.length / 4) * 4, '=')
75+
const binary = atob(padded)
76+
const bytes = new Uint8Array(binary.length)
77+
for (let index = 0; index < binary.length; index++) {
78+
bytes[index] = binary.charCodeAt(index)
79+
}
80+
return new TextDecoder().decode(bytes)
81+
} catch {
82+
return null
83+
}
84+
}
85+
86+
function getOAuthClientSigningSecret() {
87+
const secret = envFunctions.SESSION_SECRET
88+
if (!secret && process.env.NODE_ENV === 'production') {
89+
throw new Error('SESSION_SECRET is required for OAuth client registration')
90+
}
91+
92+
return secret ?? 'development-oauth-client-registration-secret'
93+
}
94+
95+
async function signOAuthClientPayload(payload: string) {
96+
const key = await crypto.subtle.importKey(
97+
'raw',
98+
new TextEncoder().encode(getOAuthClientSigningSecret()),
99+
{ name: 'HMAC', hash: 'SHA-256' },
100+
false,
101+
['sign'],
102+
)
103+
const signature = await crypto.subtle.sign(
104+
'HMAC',
105+
key,
106+
new TextEncoder().encode(payload),
107+
)
108+
return bytesToBase64Url(new Uint8Array(signature))
109+
}
110+
111+
function parseRegisteredClientPayload(
112+
value: unknown,
113+
): { clientName: string; redirectUris: Array<string> } | null {
114+
if (!isRecord(value)) return null
115+
116+
const clientName = value.clientName
117+
const redirectUris = value.redirectUris
118+
if (typeof clientName !== 'string' || !isStringArray(redirectUris)) {
119+
return null
120+
}
121+
122+
if (
123+
clientName.length > 100 ||
124+
redirectUris.length === 0 ||
125+
redirectUris.length > 10 ||
126+
redirectUris.join('').length > 4096 ||
127+
redirectUris.some((uri) => uri.length > 2048 || !validateRedirectUri(uri))
128+
) {
129+
return null
130+
}
131+
132+
return { clientName, redirectUris }
133+
}
134+
135+
export async function createRegisteredClientId(params: {
136+
clientName: string
137+
redirectUris: Array<string>
138+
}) {
139+
const payload = JSON.stringify({
140+
clientName: params.clientName,
141+
redirectUris: params.redirectUris
142+
.map(normalizeRedirectUri)
143+
.filter((uri) => uri !== null),
144+
})
145+
const encodedPayload = stringToBase64Url(payload)
146+
const signature = await signOAuthClientPayload(encodedPayload)
147+
148+
return `${REGISTERED_CLIENT_PREFIX}${encodedPayload}.${signature}`
149+
}
150+
151+
export async function validateOAuthClientRedirectUri(
152+
clientId: string,
153+
redirectUri: string,
154+
) {
155+
if (!clientId.startsWith(REGISTERED_CLIENT_PREFIX)) {
156+
return false
157+
}
158+
159+
const body = clientId.slice(REGISTERED_CLIENT_PREFIX.length)
160+
const [encodedPayload, signature] = body.split('.')
161+
if (
162+
!encodedPayload ||
163+
!signature ||
164+
encodedPayload.length > MAX_REGISTERED_CLIENT_PAYLOAD_LENGTH
165+
) {
166+
return false
167+
}
168+
169+
const expectedSignature = await signOAuthClientPayload(encodedPayload)
170+
if (expectedSignature !== signature) {
171+
return false
172+
}
173+
174+
const payloadText = base64UrlToString(encodedPayload)
175+
if (!payloadText) {
176+
return false
177+
}
178+
179+
let payload: unknown
180+
try {
181+
payload = JSON.parse(payloadText)
182+
} catch {
183+
return false
184+
}
185+
186+
const metadata = parseRegisteredClientPayload(payload)
187+
const normalizedRedirectUri = normalizeRedirectUri(redirectUri)
188+
return (
189+
!!metadata &&
190+
!!normalizedRedirectUri &&
191+
metadata.redirectUris.includes(normalizedRedirectUri)
192+
)
193+
}
194+
36195
/**
37196
* Generate a secure random token with prefix
38197
*/
@@ -56,14 +215,17 @@ export const hashToken = sha256Hex
56215
export function validateRedirectUri(uri: string): boolean {
57216
try {
58217
const url = new URL(uri)
218+
if (url.username || url.password) {
219+
return false
220+
}
59221

60222
// Allow localhost (any port)
61223
if (
62224
url.hostname === 'localhost' ||
63225
url.hostname === '127.0.0.1' ||
64226
url.hostname === '[::1]'
65227
) {
66-
return true
228+
return url.protocol === 'http:' || url.protocol === 'https:'
67229
}
68230

69231
// Allow HTTPS URLs
@@ -130,6 +292,12 @@ export async function createAuthorizationCode(params: {
130292
codeChallengeMethod?: string
131293
scope?: string
132294
}): Promise<string> {
295+
if (
296+
!(await validateOAuthClientRedirectUri(params.clientId, params.redirectUri))
297+
) {
298+
throw new Error('Client is not registered for this redirect URI')
299+
}
300+
133301
const code = generateToken('')
134302
const codeHash = await hashToken(code)
135303
const expiresAt = new Date(Date.now() + AUTH_CODE_TTL_MS)
@@ -167,12 +335,12 @@ export async function exchangeAuthorizationCode(params: {
167335
> {
168336
const codeHash = await hashToken(params.code)
169337

170-
// Find and validate auth code
338+
// Atomically consume the authorization code before validation. Any exchange
339+
// attempt makes the code single-use, even when the verifier is wrong.
171340
const authCodes = await db
172-
.select()
173-
.from(oauthAuthorizationCodes)
341+
.delete(oauthAuthorizationCodes)
174342
.where(eq(oauthAuthorizationCodes.codeHash, codeHash))
175-
.limit(1)
343+
.returning()
176344

177345
const authCode = authCodes[0]
178346

@@ -182,9 +350,6 @@ export async function exchangeAuthorizationCode(params: {
182350

183351
// Check expiration
184352
if (authCode.expiresAt < new Date()) {
185-
await db
186-
.delete(oauthAuthorizationCodes)
187-
.where(eq(oauthAuthorizationCodes.id, authCode.id))
188353
return { success: false, error: 'invalid_grant' }
189354
}
190355

@@ -202,11 +367,6 @@ export async function exchangeAuthorizationCode(params: {
202367
return { success: false, error: 'invalid_grant' }
203368
}
204369

205-
// Delete auth code (one-time use)
206-
await db
207-
.delete(oauthAuthorizationCodes)
208-
.where(eq(oauthAuthorizationCodes.id, authCode.id))
209-
210370
// Generate tokens (using new prefixes)
211371
const accessToken = generateToken(ACCESS_TOKEN_PREFIX)
212372
const refreshToken = generateToken(REFRESH_TOKEN_PREFIX)
@@ -327,6 +487,7 @@ export async function validateOAuthToken(
327487
userId: oauthAccessTokens.userId,
328488
clientId: oauthAccessTokens.clientId,
329489
expiresAt: oauthAccessTokens.expiresAt,
490+
lastUsedAt: oauthAccessTokens.lastUsedAt,
330491
})
331492
.from(oauthAccessTokens)
332493
.where(eq(oauthAccessTokens.tokenHash, tokenHash))
@@ -351,11 +512,15 @@ export async function validateOAuthToken(
351512
}
352513
}
353514

354-
// Update last used (fire and forget)
355-
db.update(oauthAccessTokens)
356-
.set({ lastUsedAt: new Date() })
357-
.where(eq(oauthAccessTokens.id, accessToken.id))
358-
.catch(() => {})
515+
if (
516+
!accessToken.lastUsedAt ||
517+
Date.now() - accessToken.lastUsedAt.getTime() > LAST_USED_WRITE_INTERVAL_MS
518+
) {
519+
db.update(oauthAccessTokens)
520+
.set({ lastUsedAt: new Date() })
521+
.where(eq(oauthAccessTokens.id, accessToken.id))
522+
.catch(() => {})
523+
}
359524

360525
return {
361526
success: true,

0 commit comments

Comments
 (0)