fix: security and correctness pass on articles, auth, storage#5
Open
basedest wants to merge 3 commits into
Open
fix: security and correctness pass on articles, auth, storage#5basedest wants to merge 3 commits into
basedest wants to merge 3 commits into
Conversation
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
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
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0c5272e1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+27
to
+30
| 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 |
There was a problem hiding this comment.
The new yarn build step is configured with only MONGODB_URI and BETTER_AUTH_SECRET, but build-time imports reach getServerConfig() (via server/context.ts → features/auth/auth.ts), and in production mode the config transform now defaults STORAGE_PROVIDER to minio and throws unless the full S3_* set is present (src/shared/config/env/server.ts). In this workflow context, that means the job can fail before the app compiles, so the added build gate is effectively broken unless storage env vars (or provider override) are supplied.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch 1 (P0) of the codebase audit. Highlights:
authenticated calls are logged
prevent ReDoS / regex injection in article.list
conflict (removes check-then-create race)
name for URLs/UI. Editor page and ArticleItem prefer authorId, fall
back to author name for legacy docs
uses the default regional endpoint); env config now exposes S3_ENDPOINT
client (no storage-config leakage)
instead of swallowing it
a placeholder