From 9dc93b3965587e9f42b49c0011ee19381ce32c55 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 18:11:38 +0000 Subject: [PATCH 1/3] fix: security and correctness pass on articles, auth, storage Batch 1 (P0) of the codebase audit. Highlights: - server/trpc.ts: protectedProcedure now chains loggingMiddleware so all authenticated calls are logged - article.repository: escape user input before regex-matching titles to prevent ReDoS / regex injection in article.list - article.service: rely on Mongo unique-index duplicate-key error for slug conflict (removes check-then-create race) - article: introduce authorId for ownership checks; keep author display name for URLs/UI. Editor page and ArticleItem prefer authorId, fall back to author name for legacy docs - article.router: drop legacy content: z.any() field; tighten validation - shared/types/session: type role as 'admin' | 'user' - storage/s3: only set endpoint when explicitly configured (real AWS S3 uses the default regional endpoint); env config now exposes S3_ENDPOINT - routers: log raw upload errors server-side; return generic message to client (no storage-config leakage) - features/auth: stop logging verification URLs (contained tokens) - shared/lib/server/connection: rethrow on MongoDB connection failure instead of swallowing it - entities/{article,user}/model: add Mongoose indexes on hot fields - shared/emails/verify-email: replace real-looking JWT preview token with a placeholder --- server/trpc.ts | 2 +- .../api/__tests__/article.service.test.ts | 70 ++++++++++++------- .../article/api/article.repository.ts | 4 +- src/entities/article/api/article.router.ts | 52 ++++++++------ src/entities/article/api/article.service.ts | 49 ++++++++----- src/entities/article/model/types.ts | 14 ++-- src/entities/article/ui/article-item.tsx | 3 +- src/entities/user/api/user.router.ts | 13 ++-- src/entities/user/model/types.ts | 4 +- src/features/auth/auth.ts | 2 +- src/shared/config/env/server.ts | 5 +- src/shared/emails/verify-email.tsx | 2 +- src/shared/lib/server/connection.ts | 14 +++- src/shared/lib/server/storage/s3.ts | 4 +- src/shared/types/session.ts | 5 +- src/views/editor/page.tsx | 13 ++-- 16 files changed, 161 insertions(+), 95 deletions(-) diff --git a/server/trpc.ts b/server/trpc.ts index e3b424e..7db2c43 100644 --- a/server/trpc.ts +++ b/server/trpc.ts @@ -59,7 +59,7 @@ export const router = t.router; export const publicProcedure = t.procedure.use(loggingMiddleware); // Protected procedure - requires authentication -export const protectedProcedure = t.procedure.use(async (opts) => { +export const protectedProcedure = t.procedure.use(loggingMiddleware).use(async (opts) => { const { ctx } = opts; if (!ctx.session || !ctx.session.user) { throw new TRPCError({ code: 'UNAUTHORIZED' }); diff --git a/src/entities/article/api/__tests__/article.service.test.ts b/src/entities/article/api/__tests__/article.service.test.ts index 2eca3de..37e5b43 100644 --- a/src/entities/article/api/__tests__/article.service.test.ts +++ b/src/entities/article/api/__tests__/article.service.test.ts @@ -85,43 +85,57 @@ describe('ArticleService', () => { ); }); - it('update allows when user is author', async () => { - const article = { ...baseArticle(), author: 'user1' }; + it('update allows when user is author by authorId', async () => { + const article = { ...baseArticle(), author: 'user1', authorId: 'id-1' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); vi.mocked(articleRepository.updateBySlug).mockResolvedValue({ ...article, title: 'Updated' } as Article); - const result = await service.update('test-slug', { title: 'Updated' }, 'user1'); + const result = await service.update('test-slug', { title: 'Updated' }, { id: 'id-1', name: 'user1' }); expect(articleRepository.updateBySlug).toHaveBeenCalledWith('test-slug', { title: 'Updated' }); expect(result).toEqual(expect.objectContaining({ title: 'Updated' })); }); + it('update falls back to author name for legacy docs without authorId', async () => { + const article = { ...baseArticle(), author: 'user1' }; + vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); + vi.mocked(articleRepository.updateBySlug).mockResolvedValue(article as Article); + + await service.update('test-slug', { title: 'X' }, { id: 'some-id', name: 'user1' }); + + expect(articleRepository.updateBySlug).toHaveBeenCalled(); + }); + it('update allows when user is admin even if not author', async () => { - const article = { ...baseArticle(), author: 'other' }; + const article = { ...baseArticle(), author: 'other', authorId: 'id-other' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); vi.mocked(articleRepository.updateBySlug).mockResolvedValue(article as Article); - await service.update('test-slug', { title: 'Updated' }, 'admin-user', 'admin'); + await service.update( + 'test-slug', + { title: 'Updated' }, + { id: 'id-admin', name: 'admin-user', role: 'admin' }, + ); expect(articleRepository.updateBySlug).toHaveBeenCalledWith('test-slug', { title: 'Updated' }); }); - it('delete allows when user is author', async () => { - const article = { ...baseArticle(), author: 'user1' }; + it('delete allows when user is author by authorId', async () => { + const article = { ...baseArticle(), author: 'user1', authorId: 'id-1' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); vi.mocked(articleRepository.deleteBySlug).mockResolvedValue(true); - const result = await service.delete('test-slug', 'user1'); + const result = await service.delete('test-slug', { id: 'id-1', name: 'user1' }); expect(result).toEqual({ success: true }); }); it('delete allows when user is admin even if not author', async () => { - const article = { ...baseArticle(), author: 'other' }; + const article = { ...baseArticle(), author: 'other', authorId: 'id-other' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); vi.mocked(articleRepository.deleteBySlug).mockResolvedValue(true); - const result = await service.delete('test-slug', 'admin-user', 'admin'); + const result = await service.delete('test-slug', { id: 'id-admin', name: 'admin-user', role: 'admin' }); expect(result).toEqual({ success: true }); }); @@ -138,34 +152,38 @@ describe('ArticleService', () => { expect(articleRepository.findBySlug).toHaveBeenCalledWith('missing'); }); - it('create throws CONFLICT when slug already exists', async () => { - vi.mocked(articleRepository.findBySlug).mockResolvedValue(baseArticle() as Article); + it('create throws CONFLICT when slug already exists (duplicate key error)', async () => { + const dupErr = Object.assign(new Error('E11000'), { code: 11000 }); + vi.mocked(articleRepository.create).mockRejectedValue(dupErr); await expect(service.create(baseArticle())).rejects.toMatchObject({ code: 'CONFLICT', message: 'Article with this slug already exists', }); - expect(articleRepository.create).not.toHaveBeenCalled(); }); }); describe('invalid input / permission', () => { - it('update throws FORBIDDEN when user is not author and not admin', async () => { - const article = { ...baseArticle(), author: 'other' }; + it('update throws FORBIDDEN when authorId does not match and not admin', async () => { + const article = { ...baseArticle(), author: 'other', authorId: 'id-other' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); - await expect(service.update('test-slug', { title: 'Updated' }, 'random-user')).rejects.toMatchObject({ + await expect( + service.update('test-slug', { title: 'X' }, { id: 'random-id', name: 'random-user' }), + ).rejects.toMatchObject({ code: 'FORBIDDEN', message: 'You do not have permission to edit this article', }); expect(articleRepository.updateBySlug).not.toHaveBeenCalled(); }); - it('update throws FORBIDDEN when user is not author and userRole is not admin', async () => { - const article = { ...baseArticle(), author: 'other' }; + it('update throws FORBIDDEN when user role is not admin and not the author', async () => { + const article = { ...baseArticle(), author: 'other', authorId: 'id-other' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); - await expect(service.update('test-slug', {}, 'random-user', 'user')).rejects.toMatchObject({ + await expect( + service.update('test-slug', {}, { id: 'random-id', name: 'random-user', role: 'user' }), + ).rejects.toMatchObject({ code: 'FORBIDDEN', }); }); @@ -173,17 +191,17 @@ describe('ArticleService', () => { it('update throws NOT_FOUND when article does not exist', async () => { vi.mocked(articleRepository.findBySlug).mockResolvedValue(null); - await expect(service.update('missing', {}, 'user1')).rejects.toMatchObject({ + await expect(service.update('missing', {}, { id: 'id-1', name: 'user1' })).rejects.toMatchObject({ code: 'NOT_FOUND', message: 'Article not found', }); }); - it('delete throws FORBIDDEN when user is not author and not admin', async () => { - const article = { ...baseArticle(), author: 'other' }; + it('delete throws FORBIDDEN when authorId does not match and not admin', async () => { + const article = { ...baseArticle(), author: 'other', authorId: 'id-other' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); - await expect(service.delete('test-slug', 'random-user')).rejects.toMatchObject({ + await expect(service.delete('test-slug', { id: 'random-id', name: 'random-user' })).rejects.toMatchObject({ code: 'FORBIDDEN', message: 'You do not have permission to delete this article', }); @@ -193,18 +211,18 @@ describe('ArticleService', () => { it('delete throws NOT_FOUND when article does not exist', async () => { vi.mocked(articleRepository.findBySlug).mockResolvedValue(null); - await expect(service.delete('missing', 'user1')).rejects.toMatchObject({ + await expect(service.delete('missing', { id: 'id-1', name: 'user1' })).rejects.toMatchObject({ code: 'NOT_FOUND', message: 'Article not found', }); }); it('delete throws INTERNAL_SERVER_ERROR when deleteBySlug returns false', async () => { - const article = { ...baseArticle(), author: 'user1' }; + const article = { ...baseArticle(), author: 'user1', authorId: 'id-1' }; vi.mocked(articleRepository.findBySlug).mockResolvedValue(article as Article); vi.mocked(articleRepository.deleteBySlug).mockResolvedValue(false); - await expect(service.delete('test-slug', 'user1')).rejects.toMatchObject({ + await expect(service.delete('test-slug', { id: 'id-1', name: 'user1' })).rejects.toMatchObject({ code: 'INTERNAL_SERVER_ERROR', message: 'Failed to delete article', }); diff --git a/src/entities/article/api/article.repository.ts b/src/entities/article/api/article.repository.ts index 094d967..d365081 100644 --- a/src/entities/article/api/article.repository.ts +++ b/src/entities/article/api/article.repository.ts @@ -28,7 +28,9 @@ export class ArticleRepository { } if (title) { - filter.title = { $regex: title, $options: 'i' }; + // Escape regex special chars to prevent ReDoS / regex injection. + const escaped = title.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + filter.title = { $regex: escaped, $options: 'i' }; } if (author) { diff --git a/src/entities/article/api/article.router.ts b/src/entities/article/api/article.router.ts index d068e24..70ce3c4 100644 --- a/src/entities/article/api/article.router.ts +++ b/src/entities/article/api/article.router.ts @@ -3,6 +3,7 @@ import { TRPCError } from '@trpc/server'; import { router, publicProcedure, protectedProcedure } from 'server/trpc'; import { articleService } from '~/entities/article/api/article.service'; import { getStorageClient } from '~/shared/lib/server/storage/factory'; +import { log } from '~/shared/lib/server/logger'; const CONTENT_TYPE_TO_EXT: Record = { 'image/jpeg': 'jpg', @@ -44,7 +45,6 @@ export const articleRouter = router({ category: z.string().min(1), img: z.string().optional(), tags: z.array(z.string()).optional(), - content: z.any().optional(), contentPm: z.record(z.string(), z.unknown()), contentFormat: z.union([z.literal('editorjs'), z.literal('pm')]).optional(), contentSchemaVersion: z.number().int().optional(), @@ -54,9 +54,14 @@ export const articleRouter = router({ }), ) .mutation(async ({ input, ctx }) => { + const userId = ctx.session.user.id; + if (!userId) { + throw new TRPCError({ code: 'BAD_REQUEST', message: 'User ID not found' }); + } return await articleService.create({ ...input, - author: ctx.session.user.name!, + author: ctx.session.user.name ?? '', + authorId: userId, }); }), @@ -70,7 +75,6 @@ export const articleRouter = router({ category: z.string().optional(), img: z.string().optional(), tags: z.array(z.string()).optional(), - content: z.any().optional(), contentPm: z.record(z.string(), z.unknown()).optional().nullable(), contentFormat: z.union([z.literal('editorjs'), z.literal('pm')]).optional(), contentSchemaVersion: z.number().int().optional(), @@ -80,22 +84,28 @@ export const articleRouter = router({ }), ) .mutation(async ({ input, ctx }) => { + const userId = ctx.session.user.id; + if (!userId) { + throw new TRPCError({ code: 'BAD_REQUEST', message: 'User ID not found' }); + } const { slug, ...updateData } = input; - return await articleService.update( - slug, - updateData, - ctx.session.user.name ?? '', - // TODO: why tf do we need to put role here? - (ctx.session.user as { role?: string }).role ?? undefined, - ); + return await articleService.update(slug, updateData, { + id: userId, + name: ctx.session.user.name ?? undefined, + role: ctx.session.user.role ?? undefined, + }); }), delete: protectedProcedure.input(z.object({ slug: z.string() })).mutation(async ({ input, ctx }) => { - return await articleService.delete( - input.slug, - ctx.session.user.name ?? '', - (ctx.session.user as { role?: string }).role ?? undefined, - ); + const userId = ctx.session.user.id; + if (!userId) { + throw new TRPCError({ code: 'BAD_REQUEST', message: 'User ID not found' }); + } + return await articleService.delete(input.slug, { + id: userId, + name: ctx.session.user.name ?? undefined, + role: ctx.session.user.role, + }); }), getAllSlugs: publicProcedure.query(async () => { @@ -137,13 +147,15 @@ export const articleRouter = router({ const url = await storage.uploadFile(buffer, key, input.contentType); return { url }; } catch (err) { - const message = err instanceof Error ? err.message : 'Storage upload failed'; + log({ + level: 'error', + message: 'cover image upload failed', + userId, + extra: { error: err instanceof Error ? err.message : String(err) }, + }); throw new TRPCError({ code: 'INTERNAL_SERVER_ERROR', - message: - message.includes('credentials') || message.includes('Storage') - ? `${message}. Configure STORAGE_PROVIDER and S3/MinIO env vars.` - : message, + message: 'Failed to upload image. Please try again.', }); } }), diff --git a/src/entities/article/api/article.service.ts b/src/entities/article/api/article.service.ts index b2aeb7c..be84bee 100644 --- a/src/entities/article/api/article.service.ts +++ b/src/entities/article/api/article.service.ts @@ -2,6 +2,13 @@ import { articleRepository, type ArticleQuery } from '~/entities/article/api/art import { type Article } from '~/entities/article/model/types'; import { TRPCError } from '@trpc/server'; +// Falls back to author-name check for legacy docs created before `authorId` existed. +function isOwnerOrAdmin(article: Article, actor: { id: string; name?: string; role?: string }): boolean { + if (actor.role === 'admin') return true; + if (article.authorId) return article.authorId === actor.id; + return !!actor.name && article.author === actor.name; +} + export class ArticleService { async findAll(query: ArticleQuery = {}) { return await articleRepository.findAll(query); @@ -19,26 +26,32 @@ export class ArticleService { } async create(articleData: Omit) { - const existing = await articleRepository.findBySlug(articleData.slug); - if (existing) { - throw new TRPCError({ - code: 'CONFLICT', - message: 'Article with this slug already exists', - }); - } - const contentFormat = articleData.contentPm != null ? 'pm' : (articleData.contentFormat ?? 'pm'); const contentSchemaVersion = contentFormat === 'pm' ? 1 : undefined; - return await articleRepository.create({ - ...articleData, - contentFormat, - contentSchemaVersion, - createdAt: new Date(), - }); + try { + return await articleRepository.create({ + ...articleData, + contentFormat, + contentSchemaVersion, + createdAt: new Date(), + }); + } catch (err) { + if (err && typeof err === 'object' && 'code' in err && (err as { code: number }).code === 11000) { + throw new TRPCError({ + code: 'CONFLICT', + message: 'Article with this slug already exists', + }); + } + throw err; + } } - async update(slug: string, updateData: Partial
, userId: string, userRole?: string) { + async update( + slug: string, + updateData: Partial
, + actor: { id: string; name?: string; role?: string }, + ) { const article = await articleRepository.findBySlug(slug); if (!article) { throw new TRPCError({ @@ -47,7 +60,7 @@ export class ArticleService { }); } - if (article.author !== userId && userRole !== 'admin') { + if (!isOwnerOrAdmin(article, actor)) { throw new TRPCError({ code: 'FORBIDDEN', message: 'You do not have permission to edit this article', @@ -57,7 +70,7 @@ export class ArticleService { return await articleRepository.updateBySlug(slug, updateData); } - async delete(slug: string, userId: string, userRole?: string) { + async delete(slug: string, actor: { id: string; name?: string; role?: string }) { const article = await articleRepository.findBySlug(slug); if (!article) { throw new TRPCError({ @@ -66,7 +79,7 @@ export class ArticleService { }); } - if (article.author !== userId && userRole !== 'admin') { + if (!isOwnerOrAdmin(article, actor)) { throw new TRPCError({ code: 'FORBIDDEN', message: 'You do not have permission to delete this article', diff --git a/src/entities/article/model/types.ts b/src/entities/article/model/types.ts index 5756eb6..1689a0f 100644 --- a/src/entities/article/model/types.ts +++ b/src/entities/article/model/types.ts @@ -11,7 +11,10 @@ export interface Article { title: string; description: string; category: string; + /** Display name of the author (used in URLs and UI). */ author: string; + /** Stable user id used for ownership/auth checks. Optional for legacy docs created before this field existed. */ + authorId?: string; createdAt: Date; editedAt?: Date; img?: string; @@ -26,14 +29,15 @@ export interface Article { } const ArticleSchema = new Schema
({ - slug: { type: String, required: true, unique: true }, + slug: { type: String, required: true, unique: true, index: true }, title: { type: String, required: true }, description: { type: String, required: true }, - category: { type: String, required: true }, - author: { type: String, required: true }, - createdAt: { type: Date, required: true }, + category: { type: String, required: true, index: true }, + author: { type: String, required: true, index: true }, + authorId: { type: String, required: false, index: true }, + createdAt: { type: Date, required: true, index: true }, img: String, - tags: [String], + tags: { type: [String], index: true }, contentPm: { type: Schema.Types.Mixed, required: false }, contentFormat: { type: String, enum: ['editorjs', 'pm'], required: false }, contentSchemaVersion: { type: Number, required: false }, diff --git a/src/entities/article/ui/article-item.tsx b/src/entities/article/ui/article-item.tsx index 6d48fea..515411d 100644 --- a/src/entities/article/ui/article-item.tsx +++ b/src/entities/article/ui/article-item.tsx @@ -43,7 +43,8 @@ export function ArticleItem(props: Article) { }); const user = session?.user as SessionUser | undefined; - const canEdit = user && (user.name === props.author || user.role === 'admin'); + const isOwner = user && (props.authorId ? props.authorId === user.id : props.author === user.name); + const canEdit = !!user && (isOwner || user.role === 'admin'); const handleDelete = () => { deleteMutation.mutate({ slug: props.slug }); diff --git a/src/entities/user/api/user.router.ts b/src/entities/user/api/user.router.ts index 1e73061..4314898 100644 --- a/src/entities/user/api/user.router.ts +++ b/src/entities/user/api/user.router.ts @@ -3,6 +3,7 @@ import { TRPCError } from '@trpc/server'; import { router, protectedProcedure } from 'server/trpc'; import { userService } from '~/entities/user/api/user.service'; import { getStorageClient } from '~/shared/lib/server/storage/factory'; +import { log } from '~/shared/lib/server/logger'; const CONTENT_TYPE_TO_EXT: Record = { 'image/jpeg': 'jpg', @@ -70,13 +71,15 @@ export const userRouter = router({ const storage = getStorageClient(); imageUrl = await storage.uploadFile(buffer, key, input.contentType); } catch (err) { - const message = err instanceof Error ? err.message : 'Storage upload failed'; + log({ + level: 'error', + message: 'avatar upload failed', + userId, + extra: { error: err instanceof Error ? err.message : String(err) }, + }); throw new TRPCError({ code: 'INTERNAL_SERVER_ERROR', - message: - message.includes('credentials') || message.includes('Storage') - ? `${message}. Configure STORAGE_PROVIDER and S3/MinIO env vars.` - : message, + message: 'Failed to upload image. Please try again.', }); } diff --git a/src/entities/user/model/types.ts b/src/entities/user/model/types.ts index 0fca90c..c87fc4c 100644 --- a/src/entities/user/model/types.ts +++ b/src/entities/user/model/types.ts @@ -13,8 +13,8 @@ export interface User { } const UserSchema = new Schema({ - name: { type: String, required: true, unique: true }, - email: { type: String, required: true, unique: true }, + name: { type: String, required: true, unique: true, index: true }, + email: { type: String, required: true, unique: true, index: true }, password: { type: String, required: true }, regDate: { type: Date, required: true }, role: String, diff --git a/src/features/auth/auth.ts b/src/features/auth/auth.ts index c3a7794..5c0289f 100644 --- a/src/features/auth/auth.ts +++ b/src/features/auth/auth.ts @@ -71,7 +71,7 @@ export const auth = betterAuth({ level: 'info', message: 'sent verification email', userId: user.id, - extra: { url, locale }, + extra: { locale }, }); }, }, diff --git a/src/shared/config/env/server.ts b/src/shared/config/env/server.ts index 8db8dbd..23234de 100644 --- a/src/shared/config/env/server.ts +++ b/src/shared/config/env/server.ts @@ -91,7 +91,8 @@ const serverEnvSchema = rawServerEnvSchema } : ({ provider: 's3' as const, - endpoint: undefined, + // Optional for AWS S3 (SDK uses regional endpoint); required for S3-compatible providers. + endpoint: raw.S3_ENDPOINT?.trim() || undefined, region: raw.S3_REGION?.trim() || '', accessKeyId: raw.S3_ACCESS_KEY?.trim() ?? '', secretAccessKey: raw.S3_SECRET_KEY?.trim() ?? '', @@ -162,7 +163,7 @@ export type ServerConfig = { } | { provider: 's3'; - endpoint: undefined; + endpoint: string | undefined; region: string; accessKeyId: string; secretAccessKey: string; diff --git a/src/shared/emails/verify-email.tsx b/src/shared/emails/verify-email.tsx index 2759779..774a28c 100644 --- a/src/shared/emails/verify-email.tsx +++ b/src/shared/emails/verify-email.tsx @@ -69,7 +69,7 @@ export default async function VerificationEmailBody({ url, locale = 'en' }: Veri } VerificationEmailBody.PreviewProps = { - url: 'https://articlify.basedest.tech/api/auth/verify-email?token=eyJhbGciOiJIUzI1NiJ9.eyJlbWFpbCI6Imkuc2hjaGVyYmFrb3ZAdmsudGVhbSIsImlhdCI6MTc3MTc0NDc2NSwiZXhwIjoxNzcxNzQ4MzY1fQ.m7GlIyJlOM6YSOtZxciLwhji2_gkbZW2dmGlyN6FGhU&callbackURL=%2F', + url: 'https://articlify.example.com/api/auth/verify-email?token=PREVIEW_TOKEN&callbackURL=%2F', locale: 'en', } as VerificationEmailBodyProps; diff --git a/src/shared/lib/server/connection.ts b/src/shared/lib/server/connection.ts index aa68a9b..309ef68 100644 --- a/src/shared/lib/server/connection.ts +++ b/src/shared/lib/server/connection.ts @@ -1,8 +1,16 @@ import mongoose from 'mongoose'; import { getServerConfig } from '~/shared/config/env/server'; +import { log } from '~/shared/lib/server/logger'; export const connectDB = async () => { - const conn = await mongoose.connect(getServerConfig().mongodb.uri).catch((err) => console.log(err)); - - return conn; + try { + return await mongoose.connect(getServerConfig().mongodb.uri); + } catch (err) { + log({ + level: 'fatal', + message: 'mongodb connection failed', + extra: { error: err instanceof Error ? err.message : String(err) }, + }); + throw err; + } }; diff --git a/src/shared/lib/server/storage/s3.ts b/src/shared/lib/server/storage/s3.ts index db09db6..af3cb8f 100644 --- a/src/shared/lib/server/storage/s3.ts +++ b/src/shared/lib/server/storage/s3.ts @@ -14,13 +14,15 @@ export class S3Storage implements StorageClient { this.bucket = config.bucket; this.publicUrl = config.publicUrl; + // For real AWS S3, leave `endpoint` undefined so the SDK uses the regional endpoint. + // For S3-compatible providers (e.g., R2, DigitalOcean Spaces), pass the API endpoint via S3_ENDPOINT. this.client = new S3Client({ region: config.region, credentials: { accessKeyId: config.accessKeyId, secretAccessKey: config.secretAccessKey, }, - endpoint: config.publicUrl, + ...(config.endpoint ? { endpoint: config.endpoint } : {}), forcePathStyle: config.forcePathStyle, }); } diff --git a/src/shared/types/session.ts b/src/shared/types/session.ts index 6dfc334..213f558 100644 --- a/src/shared/types/session.ts +++ b/src/shared/types/session.ts @@ -1,10 +1,13 @@ +/** Roles supported by Better Auth additionalFields. */ +export type UserRole = 'admin' | 'user'; + /** Session user shape (includes Better Auth additionalFields). Use for client-side typing. */ export interface SessionUser { id: string; name: string; email: string; image?: string | null; - role?: string; // TODO: should be enum + role?: UserRole; regDate?: Date; preferredLanguage?: string; username?: string | null; diff --git a/src/views/editor/page.tsx b/src/views/editor/page.tsx index 3426ecb..9876b28 100644 --- a/src/views/editor/page.tsx +++ b/src/views/editor/page.tsx @@ -180,13 +180,12 @@ export function EditorPage() { const sessionUser = session?.user as SessionUser | undefined; useEffect(() => { - if ( - editSlug && - existingArticle && - sessionUser && - existingArticle.author !== sessionUser.name && - sessionUser.role !== 'admin' - ) { + if (!editSlug || !existingArticle || !sessionUser) return; + const isAdmin = sessionUser.role === 'admin'; + const isOwner = existingArticle.authorId + ? existingArticle.authorId === sessionUser.id + : existingArticle.author === sessionUser.name; + if (!isOwner && !isAdmin) { toast({ title: tAuth('accessDenied'), description: t('noPermissionToEdit'), From aad685345d7ae63045f01e7c7f9bfa75e50e8517 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 18:35:10 +0000 Subject: [PATCH 2/3] fix: i18n, a11y, and structural improvements across UI Batch 2 (P1) of the codebase audit. i18n: - Translate previously hardcoded user-facing strings: article delete dialog, article-list empty state, multi-select Create option, mobile menu toggle label, article view dates - Add EN/RU bilingual fallback to the root not-found.tsx and global-error.tsx (both render outside the [locale] segment, so the intl provider is unavailable) - Locale-aware date formatting (article view + article-item) via next-intl useLocale/getLocale UX / a11y: - multi-select: role=combobox/listbox/option, aria-label, Escape clears input, options keyboard-activatable, configurable createLabel - header: aria-expanded, aria-controls, aria-label on mobile menu toggle - smart-list pagination: tabIndex=-1 on disabled prev/next so screen readers and keyboard users skip them - search-bar: placeholder is now required so the caller always passes a translated string (smart-list already does) - editor slug generator: accept Unicode letter classes (non-Latin/non- Cyrillic titles no longer collapse to an empty slug; falls back to a timestamped slug as a last resort) Code quality: - smart-list: drop the file-level eslint-disable react-hooks/set-state-in-effect; replace useEffect prop-syncing with the React-docs "set state during render" pattern - views/editor: split the 324-LOC page into the orchestrator (~210 LOC), use-article-edit-permission hook, and image-upload-section component - register-form: read Better Auth's structured `error.code` for the email-taken case instead of substring-matching the message text --- app/global-error.tsx | 8 +- app/not-found.tsx | 10 +- src/entities/article/ui/article-item.tsx | 23 +-- src/entities/tag/ui/tags-picker.tsx | 3 + .../auth/register/ui/register-form.tsx | 9 +- src/shared/lib/locales/en.json | 14 +- src/shared/lib/locales/ru.json | 14 +- src/shared/ui/multi-select.tsx | 50 +++++- src/views/article/page.tsx | 9 +- src/views/editor/image-upload-section.tsx | 48 ++++++ src/views/editor/page.tsx | 159 ++++++------------ .../editor/use-article-edit-permission.ts | 41 +++++ src/widgets/article-list/ui/article-list.tsx | 6 +- src/widgets/header/ui/header.tsx | 11 +- src/widgets/smart-list/ui/search-bar.tsx | 11 +- src/widgets/smart-list/ui/smart-list.tsx | 38 +++-- 16 files changed, 288 insertions(+), 166 deletions(-) create mode 100644 src/views/editor/image-upload-section.tsx create mode 100644 src/views/editor/use-article-edit-permission.ts diff --git a/app/global-error.tsx b/app/global-error.tsx index cf2b797..0fbb7f4 100644 --- a/app/global-error.tsx +++ b/app/global-error.tsx @@ -7,6 +7,8 @@ import { Button } from '~/shared/ui/button'; import { Alert, AlertDescription, AlertTitle } from '~/shared/ui/alert'; import '~/app/styles/globals.css'; +// next-intl context is unavailable above the [locale] segment, so we render +// EN + RU side-by-side rather than picking a single language for the user. export default function GlobalError({ error, reset }: { error: Error & { digest?: string }; reset: () => void }) { useEffect(() => { reportError({ @@ -22,12 +24,12 @@ export default function GlobalError({ error, reset }: { error: Error & { digest?
- Something went wrong + Something went wrong / Что-то пошло не так - {error.message || 'A critical error occurred.'} + {error.message || 'A critical error occurred. / Произошла критическая ошибка.'}
diff --git a/app/not-found.tsx b/app/not-found.tsx index dcee10e..86d9feb 100644 --- a/app/not-found.tsx +++ b/app/not-found.tsx @@ -2,20 +2,24 @@ import Link from 'next/link'; import { Button } from '~/shared/ui/button'; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '~/shared/ui/card'; +// This file only renders when the request is outside the [locale] segment +// (e.g. a static asset miss). The localized variant in app/[locale]/not-found.tsx +// handles in-app 404s. Bilingual fallback to avoid an English-only leak. export default function RootNotFound() { return (
404 - Page Not Found + Page Not Found / Страница не найдена

- The page you are looking for doesn't exist or has been moved. + The page you are looking for doesn't exist or has been moved. / Запрашиваемая страница не + существует или была перемещена.

diff --git a/src/entities/article/ui/article-item.tsx b/src/entities/article/ui/article-item.tsx index 515411d..b422887 100644 --- a/src/entities/article/ui/article-item.tsx +++ b/src/entities/article/ui/article-item.tsx @@ -3,7 +3,7 @@ import { authClient } from '~/shared/api/auth-client'; import type { SessionUser } from '~/shared/types/session'; import Image from 'next/image'; -import { useTranslations } from 'next-intl'; +import { useLocale, useTranslations } from 'next-intl'; import { Link, useRouter } from 'i18n/navigation'; import { useState } from 'react'; import type { Article } from '~/entities/article/model/types'; @@ -20,6 +20,9 @@ export function ArticleItem(props: Article) { const img = props.img ?? `/img/${props.category}.png`; const { data: session } = authClient.useSession(); const tCategory = useTranslations('category'); + const tArticles = useTranslations('articles'); + const tButton = useTranslations('button'); + const locale = useLocale(); const [dialogOpen, setDialogOpen] = useState(false); const router = useRouter(); const { toast } = useToast(); @@ -27,15 +30,15 @@ export function ArticleItem(props: Article) { const deleteMutation = trpc.article.delete.useMutation({ onSuccess: () => { toast({ - title: 'Success', - description: 'Article deleted successfully', + title: tArticles('deleteSuccessTitle'), + description: tArticles('deleteSuccessDescription'), }); router.refresh(); setDialogOpen(false); }, onError: (error) => { toast({ - title: 'Error', + title: tArticles('deleteErrorTitle'), description: error.message, variant: 'destructive', }); @@ -55,17 +58,15 @@ export function ArticleItem(props: Article) { - Delete Article - - Are you sure you want to delete this article? This action cannot be undone. - + {tArticles('deleteTitle')} + {tArticles('deleteConfirm')} @@ -105,7 +106,7 @@ export function ArticleItem(props: Article) {
@{props.author} • - {new Date(props.createdAt).toLocaleDateString()} + {new Date(props.createdAt).toLocaleDateString(locale)}
diff --git a/src/entities/tag/ui/tags-picker.tsx b/src/entities/tag/ui/tags-picker.tsx index f8dd446..b31200d 100644 --- a/src/entities/tag/ui/tags-picker.tsx +++ b/src/entities/tag/ui/tags-picker.tsx @@ -24,13 +24,16 @@ interface TagsPickerProps { export function TagsPicker({ value, onChange, defaultValue }: TagsPickerProps) { const t = useTranslations('editor'); + const tArticles = useTranslations('articles'); return ( tArticles('createTagLabel', { value: v })} /> ); } diff --git a/src/features/auth/register/ui/register-form.tsx b/src/features/auth/register/ui/register-form.tsx index ff825ed..6c3a770 100644 --- a/src/features/auth/register/ui/register-form.tsx +++ b/src/features/auth/register/ui/register-form.tsx @@ -48,7 +48,7 @@ export function RegisterForm() { password: string; name: string; username: string; - }) => Promise<{ error?: { message?: string } }> + }) => Promise<{ error?: { code?: string; message?: string } }> )({ email: formData.email, password: formData.password, @@ -57,8 +57,13 @@ export function RegisterForm() { }); if (signUpError) { + // Better Auth's `code` is the stable interface; `message` text is not. + const code = signUpError.code ?? ''; const msg = signUpError.message ?? ''; - const isEmailTaken = msg.includes('User already exists') && msg.includes('another email'); + const isEmailTaken = + code === 'USER_ALREADY_EXISTS' || + code === 'EMAIL_ALREADY_EXISTS' || + /already exists/i.test(msg); setError(isEmailTaken ? tError('emailAlreadyTaken') : msg || tError('registrationFailed')); } else { router.push('/verify-email'); diff --git a/src/shared/lib/locales/en.json b/src/shared/lib/locales/en.json index cef9ab6..6edb433 100644 --- a/src/shared/lib/locales/en.json +++ b/src/shared/lib/locales/en.json @@ -5,7 +5,8 @@ "categories": "Categories", "editor": "Editor", "signIn": "Sign In", - "brand": "Articlify" + "brand": "Articlify", + "toggleMenu": "Toggle navigation menu" }, "button": { "save": "Save", @@ -121,7 +122,16 @@ "articlesByLabel": "Articles by", "articlesByAuthor": "Articles by @{author}", "userArticlesPageTitle": "Articles by @{author} | Articlify", - "userArticlesPageDescription": "Browse all articles written by {author}" + "userArticlesPageDescription": "Browse all articles written by {author}", + "deleteTitle": "Delete Article", + "deleteConfirm": "Are you sure you want to delete this article? This action cannot be undone.", + "deleteAction": "Delete", + "deletePending": "Deleting...", + "deleteSuccessTitle": "Success", + "deleteSuccessDescription": "Article deleted successfully", + "deleteErrorTitle": "Error", + "noArticlesFound": "No articles found", + "createTagLabel": "Create \"{value}\"" }, "dashboard": { "title": "Dashboard", diff --git a/src/shared/lib/locales/ru.json b/src/shared/lib/locales/ru.json index 67fd469..649e1d9 100644 --- a/src/shared/lib/locales/ru.json +++ b/src/shared/lib/locales/ru.json @@ -5,7 +5,8 @@ "categories": "Категории", "editor": "Редактор", "signIn": "Войти", - "brand": "Articlify" + "brand": "Articlify", + "toggleMenu": "Открыть/закрыть меню" }, "button": { "save": "Сохранить", @@ -121,7 +122,16 @@ "articlesByLabel": "Статьи", "articlesByAuthor": "Статьи @{author}", "userArticlesPageTitle": "Статьи @{author} | Articlify", - "userArticlesPageDescription": "Все статьи автора {author}" + "userArticlesPageDescription": "Все статьи автора {author}", + "deleteTitle": "Удалить статью", + "deleteConfirm": "Вы уверены, что хотите удалить эту статью? Это действие нельзя отменить.", + "deleteAction": "Удалить", + "deletePending": "Удаление...", + "deleteSuccessTitle": "Готово", + "deleteSuccessDescription": "Статья успешно удалена", + "deleteErrorTitle": "Ошибка", + "noArticlesFound": "Статьи не найдены", + "createTagLabel": "Создать «{value}»" }, "dashboard": { "title": "Панель управления", diff --git a/src/shared/ui/multi-select.tsx b/src/shared/ui/multi-select.tsx index b6d18ae..cc7dbd1 100644 --- a/src/shared/ui/multi-select.tsx +++ b/src/shared/ui/multi-select.tsx @@ -17,6 +17,11 @@ interface MultiSelectProps { placeholder?: string; className?: string; allowCustom?: boolean; + ariaLabel?: string; + /** Label rendered above the "create new" option when allowCustom is true. Receives the typed value. */ + createLabel?: (value: string) => string; + /** Localized label for the per-tag remove button (e.g. "Remove tag"). */ + removeLabel?: string; } export function MultiSelect({ @@ -26,6 +31,9 @@ export function MultiSelect({ placeholder = 'Select items...', className, allowCustom = false, + ariaLabel, + createLabel, + removeLabel = 'Remove', }: MultiSelectProps) { const [inputValue, setInputValue] = React.useState(''); @@ -42,6 +50,8 @@ export function MultiSelect({ onChange([...selected, newValue]); setInputValue(''); } + } else if (e.key === 'Escape') { + setInputValue(''); } else if (e.key === 'Backspace' && !input.value && selected.length > 0) { onChange(selected.slice(0, -1)); } @@ -51,7 +61,13 @@ export function MultiSelect({ return (
-
+
0} + aria-haspopup="listbox" + aria-label={ariaLabel} + className="border-input bg-input dark:bg-input/30 ring-offset-background focus-within:ring-ring flex h-10 w-full flex-nowrap items-center gap-1 overflow-x-auto rounded-md border px-3 py-2 text-sm focus-within:ring-1" + > {selected.map((item) => { const option = options.find((o) => o.value === item); return ( @@ -59,6 +75,7 @@ export function MultiSelect({ {option?.label || item}
{inputValue && selectables.length > 0 && (
-
+
{selectables .filter((option) => option.label.toLowerCase().includes(inputValue.toLowerCase())) .map((option) => (
{ onChange([...selected, option.value]); setInputValue(''); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onChange([...selected, option.value]); + setInputValue(''); + } + }} className="hover:bg-accent hover:text-accent-foreground relative flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-none select-none" > {option.label} @@ -104,6 +135,9 @@ export function MultiSelect({ ))} {allowCustom && (
{ const newValue = inputValue.trim().toLowerCase(); if (newValue && !selected.includes(newValue)) { @@ -111,9 +145,19 @@ export function MultiSelect({ setInputValue(''); } }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const newValue = inputValue.trim().toLowerCase(); + if (newValue && !selected.includes(newValue)) { + onChange([...selected, newValue]); + setInputValue(''); + } + } + }} className="hover:bg-accent hover:text-accent-foreground relative flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-none select-none" > - Create "{inputValue}" + {createLabel ? createLabel(inputValue) : `Create "${inputValue}"`}
)}
diff --git a/src/views/article/page.tsx b/src/views/article/page.tsx index 88f1520..670f663 100644 --- a/src/views/article/page.tsx +++ b/src/views/article/page.tsx @@ -1,5 +1,5 @@ import { createServerCallerPublic } from '~/shared/api/trpc/server'; -import { getTranslations } from 'next-intl/server'; +import { getTranslations, getLocale } from 'next-intl/server'; import { notFound } from 'next/navigation'; import Image from 'next/image'; import { Link } from 'i18n/navigation'; @@ -45,6 +45,7 @@ export async function ArticlePage({ params }: ArticlePageProps) { const img = article.img ?? `/img/${article.category}.png`; const tCategory = await getTranslations('category'); const tArticles = await getTranslations('articles'); + const locale = await getLocale(); const categoryLabel = tCategory(article.category); return ( @@ -67,7 +68,7 @@ export async function ArticlePage({ params }: ArticlePageProps) { @{article.author} • - {new Date(article.createdAt).toLocaleDateString()} + {new Date(article.createdAt).toLocaleDateString(locale)}

{article.title}

@@ -87,8 +88,8 @@ export async function ArticlePage({ params }: ArticlePageProps) {

{tArticles('lastUpdated')}{' '} {article.editedAt - ? new Date(article.editedAt).toLocaleString() - : new Date(article.createdAt).toLocaleString()} + ? new Date(article.editedAt).toLocaleString(locale) + : new Date(article.createdAt).toLocaleString(locale)}

diff --git a/src/views/editor/image-upload-section.tsx b/src/views/editor/image-upload-section.tsx new file mode 100644 index 0000000..f6615c2 --- /dev/null +++ b/src/views/editor/image-upload-section.tsx @@ -0,0 +1,48 @@ +'use client'; + +import React from 'react'; +import Image from 'next/image'; +import { useTranslations } from 'next-intl'; +import { Input } from '~/shared/ui/input'; +import { Label } from '~/shared/ui/label'; + +interface ImageUploadSectionProps { + onFileChange: (file: File | null, dataUrl: string | null) => void; + displaySrc: string | null; +} + +export function ImageUploadSection({ onFileChange, displaySrc }: ImageUploadSectionProps) { + const t = useTranslations('editor'); + + const handleImageChange = (e: React.ChangeEvent) => { + const selectedFile = e.target.files?.[0]; + if (!selectedFile) { + onFileChange(null, null); + return; + } + const reader = new FileReader(); + reader.onloadend = () => onFileChange(selectedFile, reader.result as string); + reader.readAsDataURL(selectedFile); + }; + + return ( +
+ +

{t('coverImageNote')}

+
+ +
+ {displaySrc && ( +
+ {t('preview')} +
+ )} +
+ ); +} diff --git a/src/views/editor/page.tsx b/src/views/editor/page.tsx index 9876b28..0980a70 100644 --- a/src/views/editor/page.tsx +++ b/src/views/editor/page.tsx @@ -1,8 +1,7 @@ 'use client'; -import React, { useState, useEffect, useRef } from 'react'; +import React, { useState, useRef } from 'react'; import dynamic from 'next/dynamic'; -import Image from 'next/image'; import { useTranslations } from 'next-intl'; import type { ProseMirrorJSON, TiptapEditorRef } from '~/widgets/editor'; import type { Article } from '~/entities/article/model/types'; @@ -21,20 +20,34 @@ import { Card, CardContent } from '~/shared/ui/card'; import { Loader2, Save } from 'lucide-react'; import { trpc } from '~/shared/api/trpc/client'; import { useToast } from '~/shared/ui/use-toast'; +import { useArticleEditPermission } from './use-article-edit-permission'; +import { ImageUploadSection } from './image-upload-section'; const TiptapEditorDynamic = dynamic(() => import('~/widgets/editor').then((m) => ({ default: m.TiptapEditor })), { ssr: false, }); +// Accept Latin, Cyrillic, and Unicode letter classes so non-English titles +// still produce a non-empty slug. Falls back to a timestamp if nothing usable +// remains. +function getSlug(t: string): string { + const slug = t + .toLowerCase() + .trim() + .replace(/\s+/g, '-') + .replace(/[^\p{L}\p{N}-]+/gu, '') + .replace(/-{2,}/g, '-') + .replace(/^-|-$/g, ''); + return slug || `article-${Date.now()}`; +} + export function EditorPage() { const router = useRouter(); const searchParams = useSearchParams(); const editSlug = searchParams.get('edit'); const { data: session, isPending: statusPending } = authClient.useSession(); - const status = statusPending ? 'loading' : 'authenticated'; const { toast } = useToast(); const t = useTranslations('editor'); - const tAuth = useTranslations('auth'); const tCategory = useTranslations('category'); const [file, setFile] = useState(null); @@ -58,59 +71,34 @@ export function EditorPage() { editSlug && existingArticle ? { ...existingArticle, ...article } : article; const displayImageSrc = imageSrc ?? effectiveArticle.img ?? null; + useArticleEditPermission({ + editSlug, + existingArticle, + sessionUser: session?.user as SessionUser | undefined, + }); + const createMutation = trpc.article.create.useMutation({ onSuccess: (data) => { - toast({ - title: t('success'), - description: t('articleCreated'), - }); + toast({ title: t('success'), description: t('articleCreated') }); router.push(`/${data.category}/${data.slug}`); }, onError: (error) => { - toast({ - title: t('error'), - description: error.message, - variant: 'destructive', - }); + toast({ title: t('error'), description: error.message, variant: 'destructive' }); setUploading(false); }, }); const updateMutation = trpc.article.update.useMutation({ onSuccess: (data) => { - toast({ - title: t('success'), - description: t('articleUpdated'), - }); + toast({ title: t('success'), description: t('articleUpdated') }); router.push(`/${data!.category}/${data!.slug}`); }, onError: (error) => { - toast({ - title: t('error'), - description: error.message, - variant: 'destructive', - }); + toast({ title: t('error'), description: error.message, variant: 'destructive' }); setUploading(false); }, }); - const getSlug = (t: string) => { - const filter = /[^а-яё\w-]/g; - return t.toLowerCase().split(' ').join('-').replace(filter, ''); - }; - - const handleImageChange = (e: React.ChangeEvent) => { - const selectedFile = e.target.files?.[0]; - if (selectedFile) { - setFile(selectedFile); - const reader = new FileReader(); - reader.onloadend = () => { - setImageSrc(reader.result as string); - }; - reader.readAsDataURL(selectedFile); - } - }; - const uploadCoverImageMutation = trpc.article.uploadCoverImage.useMutation(); const onSubmit = async () => { @@ -141,64 +129,29 @@ export function EditorPage() { } } - const slug = getSlug(effectiveArticle.title!); + const common = { + title: effectiveArticle.title!, + description: effectiveArticle.description!, + category: effectiveArticle.category!, + tags: effectiveArticle.tags || [], + img: imageUrl, + contentPm, + contentFormat: 'pm' as const, + contentSchemaVersion: 1, + }; if (editSlug) { - updateMutation.mutate({ - slug: editSlug, - title: effectiveArticle.title!, - description: effectiveArticle.description!, - category: effectiveArticle.category!, - tags: effectiveArticle.tags, - img: imageUrl, - contentPm: contentPm, - contentFormat: 'pm', - contentSchemaVersion: 1, - }); + updateMutation.mutate({ slug: editSlug, ...common }); } else { - createMutation.mutate({ - slug, - title: effectiveArticle.title!, - description: effectiveArticle.description!, - category: effectiveArticle.category!, - tags: effectiveArticle.tags || [], - img: imageUrl, - contentPm: contentPm, - contentFormat: 'pm', - contentSchemaVersion: 1, - }); + createMutation.mutate({ slug: getSlug(effectiveArticle.title!), ...common }); } - } catch (error) { - toast({ - title: t('error'), - description: t('failedToSave'), - variant: 'destructive', - }); + } catch { + toast({ title: t('error'), description: t('failedToSave'), variant: 'destructive' }); setUploading(false); } }; - const sessionUser = session?.user as SessionUser | undefined; - useEffect(() => { - if (!editSlug || !existingArticle || !sessionUser) return; - const isAdmin = sessionUser.role === 'admin'; - const isOwner = existingArticle.authorId - ? existingArticle.authorId === sessionUser.id - : existingArticle.author === sessionUser.name; - if (!isOwner && !isAdmin) { - toast({ - title: tAuth('accessDenied'), - description: t('noPermissionToEdit'), - variant: 'destructive', - }); - router.push('/'); - } - }, [editSlug, existingArticle, sessionUser, router, toast, t, tAuth]); - - const authLoading = status === 'loading'; - const disabled = !editorReady || authLoading || uploading; - - if (authLoading || (editSlug && articleLoading)) { + if (statusPending || (editSlug && articleLoading)) { return (
@@ -206,6 +159,8 @@ export function EditorPage() { ); } + const disabled = !editorReady || uploading; + return (

{editSlug ? t('editArticle') : t('createNewArticle')}

@@ -222,7 +177,6 @@ export function EditorPage() { value={effectiveArticle.title || ''} onChange={(e) => setArticle({ ...article, title: e.target.value })} placeholder={t('placeholderTitle')} - disabled={!!editSlug} /> )}
@@ -266,24 +220,13 @@ export function EditorPage() { />
-
- -

{t('coverImageNote')}

-
- -
- {displayImageSrc && ( -
- {t('preview')} -
- )} -
+ { + setFile(f); + setImageSrc(dataUrl); + }} + />
diff --git a/src/views/editor/use-article-edit-permission.ts b/src/views/editor/use-article-edit-permission.ts new file mode 100644 index 0000000..5f1ff20 --- /dev/null +++ b/src/views/editor/use-article-edit-permission.ts @@ -0,0 +1,41 @@ +'use client'; + +import { useEffect } from 'react'; +import { useRouter } from 'i18n/navigation'; +import { useTranslations } from 'next-intl'; +import type { Article } from '~/entities/article/model/types'; +import type { SessionUser } from '~/shared/types/session'; +import { useToast } from '~/shared/ui/use-toast'; + +interface Args { + editSlug: string | null; + existingArticle: Article | null | undefined; + sessionUser: SessionUser | undefined; +} + +/** + * When editing, verifies the session user owns the article (or is admin) and + * redirects with a toast otherwise. No-op for create flow. + */ +export function useArticleEditPermission({ editSlug, existingArticle, sessionUser }: Args) { + const router = useRouter(); + const { toast } = useToast(); + const t = useTranslations('editor'); + const tAuth = useTranslations('auth'); + + useEffect(() => { + if (!editSlug || !existingArticle || !sessionUser) return; + const isAdmin = sessionUser.role === 'admin'; + const isOwner = existingArticle.authorId + ? existingArticle.authorId === sessionUser.id + : existingArticle.author === sessionUser.name; + if (!isOwner && !isAdmin) { + toast({ + title: tAuth('accessDenied'), + description: t('noPermissionToEdit'), + variant: 'destructive', + }); + router.push('/'); + } + }, [editSlug, existingArticle, sessionUser, router, toast, t, tAuth]); +} diff --git a/src/widgets/article-list/ui/article-list.tsx b/src/widgets/article-list/ui/article-list.tsx index ff49a00..baa9792 100644 --- a/src/widgets/article-list/ui/article-list.tsx +++ b/src/widgets/article-list/ui/article-list.tsx @@ -1,3 +1,4 @@ +import { useTranslations } from 'next-intl'; import type { Article } from '~/entities/article/model/types'; import { ArticleItem } from '~/entities/article/ui/article-item'; @@ -6,9 +7,12 @@ export interface ArticleListProps { } export function ArticleList({ articles }: ArticleListProps) { + const t = useTranslations('articles'); if (articles.length === 0) { return ( -
No articles found
+
+ {t('noArticlesFound')} +
); } diff --git a/src/widgets/header/ui/header.tsx b/src/widgets/header/ui/header.tsx index 572161c..e1af63d 100644 --- a/src/widgets/header/ui/header.tsx +++ b/src/widgets/header/ui/header.tsx @@ -65,14 +65,21 @@ export function Header() {
-
{mobileMenuOpen && ( -
+
setMobileMenuOpen(false)}>
@@ -212,6 +214,7 @@ export function SmartList(props: SmartListProps) { }} className={page <= 1 ? 'pointer-events-none opacity-50' : 'cursor-pointer'} aria-disabled={page <= 1} + tabIndex={page <= 1 ? -1 : undefined} /> {showLeftEllipsis && ( @@ -276,6 +279,7 @@ export function SmartList(props: SmartListProps) { }} className={page >= totalPages ? 'pointer-events-none opacity-50' : 'cursor-pointer'} aria-disabled={page >= totalPages} + tabIndex={page >= totalPages ? -1 : undefined} /> From b0c5272e1a78bef52a3379e39099fca9f5989335 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 18:37:23 +0000 Subject: [PATCH 3/3] chore: tighten tooling, CI, and storage env validation Batch 3 (P2) of the codebase audit. CI: - .github/workflows/test.yml: run yarn type-check and yarn build after lint and test so production-build regressions and TS errors that escape vitest are caught before merge ESLint: - enable @typescript-eslint/no-floating-promises and no-misused-promises (type-aware) on src/, server/, and app/ to surface unhandled async paths Vitest: - add v8 coverage config (text + json + html reporters) so `yarn test --coverage` produces measurable output Storage / env: - shared/config/env/server: MinIO fallbacks now apply only in development; production must set every S3_* var explicitly. The silent minioadmin/minioadmin default in production is gone Tests: - shared/lib/escape-regex: extract regex-escape into a reusable util and add a Vitest spec, including a ReDoS-injection regression case. article.repository now uses the util. --- .github/workflows/test.yml | 6 ++++ eslint.config.mjs | 22 +++++++++++++ .../article/api/article.repository.ts | 5 ++- src/shared/config/env/server.ts | 25 +++++++++++---- src/shared/lib/__tests__/escape-regex.test.ts | 31 +++++++++++++++++++ src/shared/lib/escape-regex.ts | 8 +++++ vitest.config.ts | 11 +++++++ 7 files changed, 99 insertions(+), 9 deletions(-) create mode 100644 src/shared/lib/__tests__/escape-regex.test.ts create mode 100644 src/shared/lib/escape-regex.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d16c22a..779de3e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,4 +21,10 @@ jobs: - run: yarn install --frozen-lockfile - run: yarn run lint + - run: yarn run type-check - run: yarn test + - run: yarn build + env: + # Build only needs schema-valid env; secrets are not exercised. + MONGODB_URI: mongodb://localhost:27017/articlify-ci + BETTER_AUTH_SECRET: ci-build-only-not-used-at-runtime diff --git a/eslint.config.mjs b/eslint.config.mjs index 3ba195f..f2f1c16 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -2,6 +2,8 @@ import { defineConfig, globalIgnores } from 'eslint/config'; import nextVitals from 'eslint-config-next/core-web-vitals'; import eslintConfigPrettier from 'eslint-config-prettier'; import prettier from 'eslint-plugin-prettier'; +import tseslint from '@typescript-eslint/eslint-plugin'; +import tsparser from '@typescript-eslint/parser'; const eslintConfig = defineConfig([ ...nextVitals, @@ -20,6 +22,26 @@ const eslintConfig = defineConfig([ 'react/display-name': 'off', }, }, + // Async-safety: catch floating promises and misused async handlers. + // Type-aware linting is required for these rules. + { + files: ['src/**/*.{ts,tsx}', 'server/**/*.ts', 'app/**/*.{ts,tsx}'], + plugins: { '@typescript-eslint': tseslint }, + languageOptions: { + parser: tsparser, + parserOptions: { + projectService: true, + tsconfigRootDir: import.meta.dirname, + }, + }, + rules: { + '@typescript-eslint/no-floating-promises': 'error', + '@typescript-eslint/no-misused-promises': [ + 'error', + { checksVoidReturn: { attributes: false } }, + ], + }, + }, // FSD layer boundaries: shared cannot import from higher layers { files: ['src/shared/**/*.ts', 'src/shared/**/*.tsx'], diff --git a/src/entities/article/api/article.repository.ts b/src/entities/article/api/article.repository.ts index d365081..fc60719 100644 --- a/src/entities/article/api/article.repository.ts +++ b/src/entities/article/api/article.repository.ts @@ -1,5 +1,6 @@ import { ArticleModel, type Article } from '~/entities/article/model/types'; import { connectDB } from '~/shared/lib/server/connection'; +import { escapeRegex } from '~/shared/lib/escape-regex'; import { Types } from 'mongoose'; export interface ArticleQuery { @@ -28,9 +29,7 @@ export class ArticleRepository { } if (title) { - // Escape regex special chars to prevent ReDoS / regex injection. - const escaped = title.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - filter.title = { $regex: escaped, $options: 'i' }; + filter.title = { $regex: escapeRegex(title), $options: 'i' }; } if (author) { diff --git a/src/shared/config/env/server.ts b/src/shared/config/env/server.ts index 23234de..d312093 100644 --- a/src/shared/config/env/server.ts +++ b/src/shared/config/env/server.ts @@ -77,16 +77,29 @@ const serverEnvSchema = rawServerEnvSchema ? raw.MAILER_RATE_LIMIT_PER_MINUTE : undefined; + // MinIO defaults match docker-compose for local dev; production must set everything explicitly. + const isProd = nodeEnv === 'production'; + const minioDefault = (envValue: string | undefined, devFallback: string, name: string): string => { + const trimmed = envValue?.trim(); + if (trimmed) return trimmed; + if (isProd) throw new Error(`${name} is required when STORAGE_PROVIDER=minio in production`); + return devFallback; + }; + const storage = storageProvider === 'minio' ? { provider: 'minio' as const, - endpoint: raw.S3_ENDPOINT?.trim() || 'http://localhost:9000', - region: raw.S3_REGION?.trim() || 'us-east-1', - accessKeyId: raw.S3_ACCESS_KEY?.trim() || 'minioadmin', - secretAccessKey: raw.S3_SECRET_KEY?.trim() || 'minioadmin', - bucket: raw.S3_BUCKET?.trim() || 'articlify-images', - publicUrl: raw.S3_PUBLIC_URL?.trim() || 'http://localhost:9000/articlify-images', + endpoint: minioDefault(raw.S3_ENDPOINT, 'http://localhost:9000', 'S3_ENDPOINT'), + region: minioDefault(raw.S3_REGION, 'us-east-1', 'S3_REGION'), + accessKeyId: minioDefault(raw.S3_ACCESS_KEY, 'minioadmin', 'S3_ACCESS_KEY'), + secretAccessKey: minioDefault(raw.S3_SECRET_KEY, 'minioadmin', 'S3_SECRET_KEY'), + bucket: minioDefault(raw.S3_BUCKET, 'articlify-images', 'S3_BUCKET'), + publicUrl: minioDefault( + raw.S3_PUBLIC_URL, + 'http://localhost:9000/articlify-images', + 'S3_PUBLIC_URL', + ), forcePathStyle: true, } : ({ diff --git a/src/shared/lib/__tests__/escape-regex.test.ts b/src/shared/lib/__tests__/escape-regex.test.ts new file mode 100644 index 0000000..8efd933 --- /dev/null +++ b/src/shared/lib/__tests__/escape-regex.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from 'vitest'; +import { escapeRegex } from '~/shared/lib/escape-regex'; + +describe('escapeRegex', () => { + it('leaves ordinary characters untouched', () => { + expect(escapeRegex('hello world')).toBe('hello world'); + expect(escapeRegex('Привет мир')).toBe('Привет мир'); + }); + + it('escapes all regex metacharacters', () => { + expect(escapeRegex('.*+?^${}()|[]\\')).toBe('\\.\\*\\+\\?\\^\\$\\{\\}\\(\\)\\|\\[\\]\\\\'); + }); + + it('produces a regex source that matches the original input literally', () => { + const inputs = ['a(b', 'foo.*bar', '$100', '(^_^)', 'C:\\path\\to\\file']; + for (const s of inputs) { + const re = new RegExp(escapeRegex(s)); + expect(re.test(s)).toBe(true); + } + }); + + it('escaped input cannot trigger ReDoS by injecting alternation/quantifiers', () => { + // Without escaping, `(a+)+` triggers catastrophic backtracking on input + // like 'aaaaaaaaaaaaaaaaa!'. With escaping, the source matches literally + // and the regex compiles to a safe literal. + const evil = '(a+)+'; + const re = new RegExp(escapeRegex(evil)); + expect(re.test(evil)).toBe(true); + expect(re.test('aaaaaaaaaaaaaaaaa!')).toBe(false); + }); +}); diff --git a/src/shared/lib/escape-regex.ts b/src/shared/lib/escape-regex.ts new file mode 100644 index 0000000..b145a71 --- /dev/null +++ b/src/shared/lib/escape-regex.ts @@ -0,0 +1,8 @@ +/** + * Escape a string so it can be safely used inside a RegExp literal source. + * Use this when accepting user-controlled input into a regex (e.g. case-insensitive + * substring search in a database query). + */ +export function escapeRegex(input: string): string { + return input.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} diff --git a/vitest.config.ts b/vitest.config.ts index ed456ad..6ed5d27 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -8,6 +8,17 @@ export default defineConfig({ setupFiles: ['src/shared/test/setup.ts'], include: ['src/**/*.test.{ts,tsx}'], exclude: ['node_modules', '.next', '**/node_modules/**'], + coverage: { + provider: 'v8', + reporter: ['text', 'json', 'html'], + include: ['src/**/*.{ts,tsx}'], + exclude: [ + 'src/**/*.test.{ts,tsx}', + 'src/**/__tests__/**', + 'src/shared/test/**', + 'src/**/*.d.ts', + ], + }, }, resolve: { alias: {