Server hardening#454
Merged
Merged
Conversation
…tch lookup, and trpc caching
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe changes centralize mobile API authentication, CORS, and tRPC caching; add bounded mobile schemas and a shared Steam batch lookup service; update admin lookup handling and generated API documentation; and adjust query refresh, catalog cache, sitemap, and test setup behavior. ChangesMobile API and Steam batch lookup
Application maintenance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BatchSteamLookup
participant titleIdToolsRouter
participant lookupGamesBySteamAppIds
participant steamBatchQueryCache
participant GamesRepository
BatchSteamLookup->>titleIdToolsRouter: submit batchSteamAppIds query
titleIdToolsRouter->>lookupGamesBySteamAppIds: pass input and Prisma context
lookupGamesBySteamAppIds->>steamBatchQueryCache: read batch cache
lookupGamesBySteamAppIds->>GamesRepository: fetch matched games and listings
GamesRepository-->>lookupGamesBySteamAppIds: return ordered results
lookupGamesBySteamAppIds-->>titleIdToolsRouter: return batch response
titleIdToolsRouter-->>BatchSteamLookup: render normalized rows
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/admin/components/ApprovalCountBadge.tsx`:
- Around line 33-49: Remove the explicit refetchOnMount and refetchOnWindowFocus
options from both listingStatsQuery and pcListingStatsQuery in
ApprovalCountBadge, preserving staleTime: CACHE_DURATIONS.SHORT and the existing
enabled conditions so React Query can refresh stale approval counts on mount and
window focus.
In `@src/app/admin/title-id-tools/components/BatchSteamLookup.tsx`:
- Around line 154-158: Update the copied-results summary in the BatchSteamLookup
result-formatting logic to replace the user-facing “listings” wording with the
appropriate report terminology, including the unavailable case. Preserve the
existing Steam app ID, title, and match-strategy formatting.
In `@src/proxy.ts`:
- Around line 81-84: Restrict the no-origin bypass in hasRequestOriginMetadata
and the mobile endpoint handling to the dedicated /api/mobile/trpc/* route. Do
not let isMobileTRPCPath match /api/trpc/mobile.* requests for this relaxation;
preserve their existing origin validation or INTERNAL_API_KEY fallback,
including mutations and mixed batches.
In `@src/scripts/api/generate-api-docs.ts`:
- Around line 40-81: Update getResponseExampleOverride for
BatchBySteamAppIdsResponseSchema to represent the default minimal: false
response shape, including steamAppId, game, matchStrategy, and nested
system/listings fields; alternatively clearly label it as the minimal: true
variant. Regenerate public/api-docs/mobile-openapi.json at lines 7655-7689 so
the endpoint success example matches the corrected default shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ddb9018f-f253-4cdf-a515-bf75a3f283a7
📒 Files selected for processing (31)
docs/MOBILE_API.mdnext.config.tspublic/api-docs/mobile-openapi.jsonsrc/app/admin/components/ApprovalCountBadge.test.tsxsrc/app/admin/components/ApprovalCountBadge.tsxsrc/app/admin/custom-field-templates/page.test.tsxsrc/app/admin/title-id-tools/components/BatchSteamLookup.tsxsrc/app/api/mobile/trpc/[trpc]/route.tssrc/app/api/trpc/[trpc]/route.tssrc/app/listings/[id]/components/EditListingButton.tsxsrc/app/pc-listings/[id]/components/EditPcListingButton.tsxsrc/app/sitemap.tssrc/lib/api.tsxsrc/lib/cors.test.tssrc/lib/cors.tssrc/proxy.test.tssrc/proxy.tssrc/schemas/mobile.tssrc/scripts/api/generate-api-docs.tssrc/server/api/mobileContext.test.tssrc/server/api/mobileContext.tssrc/server/api/routers/admin/titleIdTools.tssrc/server/api/routers/mobile/catalog.tssrc/server/api/routers/mobile/games.tssrc/server/api/trpc-cache.test.tssrc/server/api/trpc-cache.tssrc/server/services/catalog.service.test.tssrc/server/services/catalog.service.tssrc/server/services/steam-batch-lookup.service.test.tssrc/server/services/steam-batch-lookup.service.tssrc/server/utils/cache/instances.ts
💤 Files with no reviewable changes (1)
- next.config.ts
Comment on lines
+33
to
+49
| staleTime: CACHE_DURATIONS.SHORT, | ||
| refetchOnMount: false, | ||
| refetchOnWindowFocus: false, | ||
| }) | ||
|
|
||
| const listingStatsQuery = api.listings.stats.useQuery(undefined, { | ||
| enabled: canViewStats && props.href === '/admin/approvals', | ||
| refetchInterval: POLLING_INTERVALS.SHORT, | ||
| staleTime: CACHE_DURATIONS.VERY_SHORT, | ||
| refetchOnMount: true, | ||
| refetchOnWindowFocus: true, | ||
| staleTime: CACHE_DURATIONS.SHORT, | ||
| refetchOnMount: false, | ||
| refetchOnWindowFocus: false, | ||
| }) | ||
|
|
||
| const pcListingStatsQuery = api.pcListings.stats.useQuery(undefined, { | ||
| enabled: canViewStats && props.href === '/admin/pc-listing-approvals', | ||
| refetchInterval: POLLING_INTERVALS.SHORT, | ||
| staleTime: CACHE_DURATIONS.VERY_SHORT, | ||
| refetchOnMount: true, | ||
| refetchOnWindowFocus: true, | ||
| staleTime: CACHE_DURATIONS.SHORT, | ||
| refetchOnMount: false, | ||
| refetchOnWindowFocus: false, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove refetchOnMount and refetchOnWindowFocus to allow stale data to refresh.
By setting refetchOnMount: false and refetchOnWindowFocus: false, these queries will never automatically refetch once they have initial data, even after the staleTime has expired. The staleTime: CACHE_DURATIONS.SHORT setting alone is sufficient to prevent unnecessary requests while the data is fresh.
Consider removing these explicit false flags. This allows React Query's default behavior to fetch updated approval counts when a user navigates back to the admin area or refocuses the window after the cache has become stale.
♻️ Proposed fix
const gameStatsQuery = api.games.stats.useQuery(undefined, {
enabled: canViewStats && props.href === '/admin/games/approvals',
staleTime: CACHE_DURATIONS.SHORT,
- refetchOnMount: false,
- refetchOnWindowFocus: false,
})
const listingStatsQuery = api.listings.stats.useQuery(undefined, {
enabled: canViewStats && props.href === '/admin/approvals',
staleTime: CACHE_DURATIONS.SHORT,
- refetchOnMount: false,
- refetchOnWindowFocus: false,
})
const pcListingStatsQuery = api.pcListings.stats.useQuery(undefined, {
enabled: canViewStats && props.href === '/admin/pc-listing-approvals',
staleTime: CACHE_DURATIONS.SHORT,
- refetchOnMount: false,
- refetchOnWindowFocus: false,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| staleTime: CACHE_DURATIONS.SHORT, | |
| refetchOnMount: false, | |
| refetchOnWindowFocus: false, | |
| }) | |
| const listingStatsQuery = api.listings.stats.useQuery(undefined, { | |
| enabled: canViewStats && props.href === '/admin/approvals', | |
| refetchInterval: POLLING_INTERVALS.SHORT, | |
| staleTime: CACHE_DURATIONS.VERY_SHORT, | |
| refetchOnMount: true, | |
| refetchOnWindowFocus: true, | |
| staleTime: CACHE_DURATIONS.SHORT, | |
| refetchOnMount: false, | |
| refetchOnWindowFocus: false, | |
| }) | |
| const pcListingStatsQuery = api.pcListings.stats.useQuery(undefined, { | |
| enabled: canViewStats && props.href === '/admin/pc-listing-approvals', | |
| refetchInterval: POLLING_INTERVALS.SHORT, | |
| staleTime: CACHE_DURATIONS.VERY_SHORT, | |
| refetchOnMount: true, | |
| refetchOnWindowFocus: true, | |
| staleTime: CACHE_DURATIONS.SHORT, | |
| refetchOnMount: false, | |
| refetchOnWindowFocus: false, | |
| const gameStatsQuery = api.games.stats.useQuery(undefined, { | |
| enabled: canViewStats && props.href === '/admin/games/approvals', | |
| staleTime: CACHE_DURATIONS.SHORT, | |
| }) | |
| const listingStatsQuery = api.listings.stats.useQuery(undefined, { | |
| enabled: canViewStats && props.href === '/admin/approvals', | |
| staleTime: CACHE_DURATIONS.SHORT, | |
| }) | |
| const pcListingStatsQuery = api.pcListings.stats.useQuery(undefined, { | |
| enabled: canViewStats && props.href === '/admin/pc-listing-approvals', | |
| staleTime: CACHE_DURATIONS.SHORT, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/components/ApprovalCountBadge.tsx` around lines 33 - 49, Remove
the explicit refetchOnMount and refetchOnWindowFocus options from both
listingStatsQuery and pcListingStatsQuery in ApprovalCountBadge, preserving
staleTime: CACHE_DURATIONS.SHORT and the existing enabled conditions so React
Query can refresh stale approval counts on mount and window focus.
Comment on lines
+154
to
+158
| const listingSummary = | ||
| displayResult.listingCount === null | ||
| ? 'listings unavailable' | ||
| : `${displayResult.listingCount} listings` | ||
| return `${displayResult.steamAppId}: ${displayResult.title} (${displayResult.matchStrategy}, ${listingSummary})` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
User-facing wording uses "listings". As per coding guidelines ("User-facing UI should use 'Compatibility Report', 'Handheld Report', 'PC Report', or 'PC Compatibility Report', not 'listing'"), the copied-results summary should use the report terminology rather than "listings".
Proposed wording change
- const listingSummary =
- displayResult.listingCount === null
- ? 'listings unavailable'
- : `${displayResult.listingCount} listings`
+ const listingSummary =
+ displayResult.listingCount === null
+ ? 'reports unavailable'
+ : `${displayResult.listingCount} compatibility reports`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const listingSummary = | |
| displayResult.listingCount === null | |
| ? 'listings unavailable' | |
| : `${displayResult.listingCount} listings` | |
| return `${displayResult.steamAppId}: ${displayResult.title} (${displayResult.matchStrategy}, ${listingSummary})` | |
| const listingSummary = | |
| displayResult.listingCount === null | |
| ? 'reports unavailable' | |
| : `${displayResult.listingCount} compatibility reports` | |
| return `${displayResult.steamAppId}: ${displayResult.title} (${displayResult.matchStrategy}, ${listingSummary})` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/admin/title-id-tools/components/BatchSteamLookup.tsx` around lines
154 - 158, Update the copied-results summary in the BatchSteamLookup
result-formatting logic to replace the user-facing “listings” wording with the
appropriate report terminology, including the unavailable case. Preserve the
existing Steam app ID, title, and match-strategy formatting.
Source: Coding guidelines
Comment on lines
+81
to
+84
| function hasRequestOriginMetadata(req: NextRequest): boolean { | ||
| return Boolean(req.headers.get('origin') || req.headers.get('referer')) | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what procedures exist under the mobile router namespace, and whether any
# are mutations (which would be reachable via /api/trpc/mobile.* without origin/API-key checks
# when Origin/Referer are absent).
fd -e ts -p 'src/server/api/routers/mobile' --exec ast-grep run --pattern 'mutation($$$)' --lang typescript {} \;Repository: Producdevity/EmuReady
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== proxy.ts relevant lines =="
nl -ba src/proxy.ts | sed -n '1,240p'
echo
echo "== locate mobile router files =="
fd -a 'mobile' src/server/api/routers || true
echo
echo "== search for mobile procedures and router definitions =="
rg -n "router\\(|mutation\\(|query\\(|mobile\\." src/server/api/routers src/server/api -g '*.ts' -g '*.tsx' || trueRepository: Producdevity/EmuReady
Length of output: 229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== proxy.ts relevant lines =="
cat -n src/proxy.ts | sed -n '1,240p'
echo
echo "== repo files for mobile router =="
git ls-files 'src/server/api/routers/**' | sed -n '/mobile/p'
echo
echo "== mobile-related procedures =="
rg -n "mobile\\.|router\\(|mutation\\(|query\\(" src/server/api/routers src/server/api -g '*.ts' -g '*.tsx' || trueRepository: Producdevity/EmuReady
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app router / mobile router composition =="
git ls-files 'src/server/api/*.ts' 'src/server/api/**/*.ts' | rg 'src/server/api/(root|trpc|routers/mobile)\.ts$' || true
echo
echo "== outline of root router file =="
ast-grep outline src/server/api/root.ts --view expanded || true
echo
echo "== outline of mobile router file =="
ast-grep outline src/server/api/routers/mobile.ts --view expanded || true
echo
echo "== read relevant slices =="
for f in src/server/api/root.ts src/server/api/routers/mobile.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: Producdevity/EmuReady
Length of output: 7934
Scope the no-origin bypass to the dedicated mobile endpoint (src/proxy.ts:112-164)
isMobileTRPCPath also matches /api/trpc/mobile.*, so requests to the web tRPC namespace skip origin validation whenever Origin/Referer is missing. That widens the relaxation to the whole mobile router, including mutations and mixed batches. Keep this bypass limited to /api/mobile/trpc/*, or preserve the INTERNAL_API_KEY fallback for /api/trpc/mobile.*.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/proxy.ts` around lines 81 - 84, Restrict the no-origin bypass in
hasRequestOriginMetadata and the mobile endpoint handling to the dedicated
/api/mobile/trpc/* route. Do not let isMobileTRPCPath match /api/trpc/mobile.*
requests for this relaxation; preserve their existing origin validation or
INTERNAL_API_KEY fallback, including mutations and mixed batches.
Comment on lines
+40
to
+81
| function getResponseExampleOverride(outputSchemaName: string | undefined): unknown | null { | ||
| if (outputSchemaName !== 'BatchBySteamAppIdsResponseSchema') return null | ||
|
|
||
| return { | ||
| success: true, | ||
| results: [ | ||
| { | ||
| game_id: 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx', | ||
| steam_app_id: '220', | ||
| title: 'Half-Life 2', | ||
| performance: { | ||
| id: 1, | ||
| label: 'Perfect', | ||
| rank: 1, | ||
| description: null, | ||
| }, | ||
| emulator: { | ||
| id: 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx', | ||
| name: 'GameHub', | ||
| logo: null, | ||
| }, | ||
| device: { | ||
| id: 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx', | ||
| modelName: 'Steam Deck', | ||
| soc: null, | ||
| }, | ||
| listing: { | ||
| id: 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx', | ||
| notes: 'Runs well', | ||
| upvoteCount: 4, | ||
| downvoteCount: 1, | ||
| voteCount: 5, | ||
| successRate: 0.8, | ||
| }, | ||
| }, | ||
| ], | ||
| totalRequested: 1, | ||
| totalFound: 1, | ||
| totalNotFound: 0, | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Batch lookup success example documents the minimal result shape, but the default response is the full shape. BatchBySteamAppIdsSchema.minimal defaults to false, so default calls return full results (steamAppId, game, matchStrategy, nested system/listings), not the minimal shape shown.
src/scripts/api/generate-api-docs.ts#L40-L81: changegetResponseExampleOverrideto emit a full-shape example (matchingminimal: false), or annotate it as theminimal: truevariant.public/api-docs/mobile-openapi.json#L7655-L7689: regenerate so the endpoint's success example reflects the corrected default shape.
📍 Affects 2 files
src/scripts/api/generate-api-docs.ts#L40-L81(this comment)public/api-docs/mobile-openapi.json#L7655-L7689
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/scripts/api/generate-api-docs.ts` around lines 40 - 81, Update
getResponseExampleOverride for BatchBySteamAppIdsResponseSchema to represent the
default minimal: false response shape, including steamAppId, game,
matchStrategy, and nested system/listings fields; alternatively clearly label it
as the minimal: true variant. Regenerate public/api-docs/mobile-openapi.json at
lines 7655-7689 so the endpoint success example matches the corrected default
shape.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo Producdevity/EmuReady \
--title "Document the default full Steam batch lookup response example" \
--assignee "Producdevity" \
--body "$(cat <<'EOF'
## Summary
The generated OpenAPI success example for `games.batchBySteamAppIds` currently shows the minimal result variant, while `BatchBySteamAppIdsSchema.minimal` defaults to `false` and the default response is the full result shape.
## Required changes
- Update `getResponseExampleOverride` in `src/scripts/api/generate-api-docs.ts` so the `BatchBySteamAppIdsResponseSchema` example represents the default full response shape.
- Include the full-result fields such as `steamAppId`, `game`, `matchStrategy`, and the nested `system` and `listings` data.
- Alternatively, explicitly label the current example as applying to `minimal: true`.
- Regenerate `public/api-docs/mobile-openapi.json` so the documented endpoint success example matches the selected behavior.
## Rationale
API consumers should be able to rely on the default request/response examples. Documenting the minimal response as the default can lead clients to model the wrong contract.
## Affected areas
- `src/scripts/api/generate-api-docs.ts`
- `public/api-docs/mobile-openapi.json`
## Acceptance criteria
- The generated example accurately reflects `minimal: false`, or is unambiguously documented as a `minimal: true` example.
- The OpenAPI JSON is regenerated from the updated generator.
- The endpoint documentation and response schema/example are consistent.
## Backlinks
- Pull request: https://github.com/Producdevity/EmuReady/pull/454
- Review comment: https://github.com/Producdevity/EmuReady/pull/454#discussion_r3611376667
- Requested by: `@Producdevity`
EOF
)"Length of output: 1952
@Producdevity, created and assigned follow-up issue #457: Document the default full Steam batch lookup response example.
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
1 participant
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.
Description
Hardens the web and mobile API paths and reduces unnecessary server work.
private, no-storeAuthorization: ApiKey ...credentials while temporarily ignoring stalex-api-keyvalues so existing mobile clients can fall through to Bearer or anonymous authenticationwww.emuready.comType of change
How Has This Been Tested?
Notes
The stale
x-api-keyfallback is temporary compatibility behavior for shipped EmuReadyApp clients. Invalid keys supplied through the explicit Authorization: ApiKey ... scheme are still rejected. The fallback should be removed once the mobile app no longer sends a stalex-api-keyalongside Bearer authentication. This was a fuck up I made, i will likely wait for most users to update instead of forcing the app update (this is still in progress).Public tRPC caching is intentionally limited to named anonymous GET queries. Requests containing cookies, authorization headers, API keys, errors, batches, mutations, or authenticated context remain private, no-store.
Summary by CodeRabbit
x-api-keyheader.