Acts on an adversarial review of the M1 auth and authorization code. Five findings fixed; the rest recorded in SECURITY-FINDINGS.md as the M1 exit criteria rather than left in a tool transcript. - auth: serve /phone-number/request-password-reset, /phone-number/ reset-password and /sign-in/phone-number as 404. better-auth's phoneNumber() registers all three unconditionally -- they are NOT gated on emailAndPassword.enabled:false. Left live they form a silent second credential path: request-password-reset stores an OTP and sends no SMS (sendPasswordResetOTP was never configured, so the owner is never told), reset-password mints a bcrypt credential row, and sign-in/phone-number then accepts it forever with no OTP. The OTP gate still applies, so this is not remote unauthenticated takeover -- it converts one momentary OTP compromise into permanent access the victim cannot see or rotate. - auth: drop bearer(). It accepts the plaintext sessions.token column as an Authorization credential, making any single leaked row a replayable login. The mobile client it was added for is hypothetical. - auth: pin E.164 via phoneNumberValidator, and add toE164/isE164 to @linkder/shared. phone is UNIQUE and bans are per-account, so "+34600111222" and "0034600111222" being separately storable meant one handset could hold two accounts and a ban was escapable by retyping. 15 tests. - auth: NEXT_PUBLIC_APP_URL now throws in production instead of falling back to localhost, which was silently dropping Secure and the __Secure- prefix from the production session cookie. - pro.upsertProfile: actually apply requiresReReview. It was computed, returned to the client and never acted on, so a verified plumber could become a verified electrician in another city by ignoring a response flag. Now demotes to pending in the same transaction and audits it. Trade changes count as material (they did not before) -- the licence is per-trade. Needed a verified -> pending edge in VERIFICATION_GRAPH, which did not exist. Removed two untracked scratch repro files. The impersonation repro depended on bearer() for transport and no longer applies as written; the underlying finding (resolveSession drops impersonatedBy, so admin actions are audited as the victim) is open and documented. typecheck, lint, build clean; 127 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
179 lines
5.8 KiB
TypeScript
179 lines
5.8 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
|
import {
|
|
StorageError,
|
|
UPLOAD_KINDS,
|
|
buildKey,
|
|
isPrivateKind,
|
|
resetStorageClient,
|
|
validateUpload,
|
|
} from '../src/index';
|
|
|
|
const OWNER = '11111111-2222-4333-8444-555555555555';
|
|
|
|
describe('validateUpload', () => {
|
|
it('accepts an image for a photo upload', () => {
|
|
expect(() =>
|
|
validateUpload({ kind: 'pro_photo', contentType: 'image/jpeg', contentLength: 1024 }),
|
|
).not.toThrow();
|
|
});
|
|
|
|
it('rejects a content type that is not on the allow list', () => {
|
|
expect(() =>
|
|
validateUpload({ kind: 'pro_photo', contentType: 'text/html', contentLength: 1024 }),
|
|
).toThrow(StorageError);
|
|
});
|
|
|
|
it('rejects an SVG — it can carry script', () => {
|
|
expect(() =>
|
|
validateUpload({ kind: 'avatar', contentType: 'image/svg+xml', contentLength: 1024 }),
|
|
).toThrow(StorageError);
|
|
});
|
|
|
|
it('rejects a file over the per-kind cap', () => {
|
|
expect(() =>
|
|
validateUpload({
|
|
kind: 'avatar',
|
|
contentType: 'image/png',
|
|
contentLength: UPLOAD_KINDS.avatar.maxBytes + 1,
|
|
}),
|
|
).toThrow(/too large/i);
|
|
});
|
|
|
|
it('accepts a file exactly on the cap', () => {
|
|
expect(() =>
|
|
validateUpload({
|
|
kind: 'avatar',
|
|
contentType: 'image/png',
|
|
contentLength: UPLOAD_KINDS.avatar.maxBytes,
|
|
}),
|
|
).not.toThrow();
|
|
});
|
|
|
|
it('allows PDFs for credentials but not for avatars', () => {
|
|
expect(() =>
|
|
validateUpload({ kind: 'credential', contentType: 'application/pdf', contentLength: 1024 }),
|
|
).not.toThrow();
|
|
expect(() =>
|
|
validateUpload({ kind: 'avatar', contentType: 'application/pdf', contentLength: 1024 }),
|
|
).toThrow(StorageError);
|
|
});
|
|
|
|
it('names the allowed types in the error so the UI can show it', () => {
|
|
try {
|
|
validateUpload({ kind: 'avatar', contentType: 'video/mp4', contentLength: 10 });
|
|
throw new Error('should have thrown');
|
|
} catch (error) {
|
|
expect((error as Error).message).toMatch(/image\/jpeg/);
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('buildKey', () => {
|
|
it('puts the owner in the path and the right extension on the end', () => {
|
|
const key = buildKey('credential', OWNER, 'application/pdf');
|
|
expect(key).toMatch(new RegExp(`^credentials/${OWNER}/[0-9a-f-]{36}\\.pdf$`));
|
|
});
|
|
|
|
it('never collides across two calls', () => {
|
|
const a = buildKey('pro_photo', OWNER, 'image/png');
|
|
const b = buildKey('pro_photo', OWNER, 'image/png');
|
|
expect(a).not.toBe(b);
|
|
});
|
|
|
|
it("keeps one owner out of another owner's prefix", () => {
|
|
const mine = buildKey('credential', OWNER, 'image/png');
|
|
const theirs = buildKey('credential', '99999999-2222-4333-8444-555555555555', 'image/png');
|
|
expect(mine.startsWith(`credentials/${OWNER}/`)).toBe(true);
|
|
expect(theirs.startsWith(`credentials/${OWNER}/`)).toBe(false);
|
|
});
|
|
|
|
it('falls back to .bin for an unmapped type rather than producing a bare key', () => {
|
|
// validateUpload is the gate; buildKey must still not emit an extensionless key.
|
|
expect(buildKey('pro_photo', OWNER, 'application/octet-stream')).toMatch(/\.bin$/);
|
|
});
|
|
});
|
|
|
|
describe('isPrivateKind', () => {
|
|
it('marks credentials private and photos public', () => {
|
|
expect(isPrivateKind('credential')).toBe(true);
|
|
expect(isPrivateKind('pro_photo')).toBe(false);
|
|
expect(isPrivateKind('avatar')).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('configuration', () => {
|
|
const saved = { ...process.env };
|
|
|
|
beforeEach(() => {
|
|
resetStorageClient();
|
|
for (const k of [
|
|
'R2_ACCOUNT_ID',
|
|
'R2_ACCESS_KEY_ID',
|
|
'R2_SECRET_ACCESS_KEY',
|
|
'R2_BUCKET',
|
|
'R2_PUBLIC_URL',
|
|
]) {
|
|
delete process.env[k];
|
|
}
|
|
});
|
|
|
|
afterEach(() => {
|
|
process.env = { ...saved };
|
|
resetStorageClient();
|
|
});
|
|
|
|
it('names every missing variable instead of failing vaguely', async () => {
|
|
const { createPresignedUpload } = await import('../src/index');
|
|
await expect(
|
|
createPresignedUpload({
|
|
kind: 'avatar',
|
|
contentType: 'image/png',
|
|
contentLength: 100,
|
|
ownerId: OWNER,
|
|
}),
|
|
).rejects.toThrow(/R2_ACCOUNT_ID.*R2_ACCESS_KEY_ID/s);
|
|
});
|
|
|
|
it('validates the upload before it complains about configuration', async () => {
|
|
const { createPresignedUpload } = await import('../src/index');
|
|
// A bad content type is the caller's fault and should be reported as such,
|
|
// even on a machine with no R2 credentials.
|
|
await expect(
|
|
createPresignedUpload({
|
|
kind: 'avatar',
|
|
contentType: 'text/html',
|
|
contentLength: 100,
|
|
ownerId: OWNER,
|
|
}),
|
|
).rejects.toThrow(/not allowed/i);
|
|
});
|
|
});
|
|
|
|
describe('key/URL contract with the API', () => {
|
|
/**
|
|
* Regression: credential uploads return an object KEY (they are private and
|
|
* have no public URL), but the credential schema originally demanded
|
|
* z.string().url(). The result was that pro onboarding could never be
|
|
* completed — every document upload failed validation at the last step.
|
|
*
|
|
* This asserts the contract in both directions so the two halves cannot drift
|
|
* apart again.
|
|
*/
|
|
it('produces a key that is NOT a URL for private kinds', () => {
|
|
const key = buildKey('credential', OWNER, 'application/pdf');
|
|
expect(() => new URL(key)).toThrow();
|
|
expect(isPrivateKind('credential')).toBe(true);
|
|
});
|
|
|
|
it('accepts that key against the credential schema', async () => {
|
|
const { credentialSchema } = await import('@linkder/shared');
|
|
const key = buildKey('credential', OWNER, 'application/pdf');
|
|
expect(credentialSchema.safeParse({ kind: 'insurance', fileKey: key }).success).toBe(true);
|
|
});
|
|
|
|
it('rejects an empty key rather than storing a dangling reference', async () => {
|
|
const { credentialSchema } = await import('@linkder/shared');
|
|
expect(credentialSchema.safeParse({ kind: 'insurance', fileKey: '' }).success).toBe(false);
|
|
});
|
|
});
|