[codex] Harden update enumeration and attachment reads#2057
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces an IP-based update-enumeration guard that tracks distinct missing app_id requests per client IP and returns HTTP 429 when thresholds are exceeded, plus updates default bundle version naming to use locally-scoped beta identifiers and aligns test fixture dependencies to exact versions. ChangesUpdate Enumeration Rate Limiting
Bundle Naming and Test Fixture Updates
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
d0b55e4 to
e326a52
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
supabase/functions/_backend/files/files.ts (1)
467-470: 💤 Low valueUse optional chaining for cleaner null check.
Per static analysis, the conditional can be simplified using optional chaining.
♻️ Suggested refactor
- const app = await getAppByAppIdPg(c, signedScopedPath.app_id, drizzleClient) - if (!app || app.owner_org !== signedScopedPath.owner_org) { + const app = await getAppByAppIdPg(c, signedScopedPath.app_id, drizzleClient) + if (app?.owner_org !== signedScopedPath.owner_org) { quickError(404, 'not_found', 'Not found') }🤖 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 `@supabase/functions/_backend/files/files.ts` around lines 467 - 470, Replace the two-step null and owner_org check with a single optional-chaining conditional to simplify the logic: in the block using getAppByAppIdPg and quickError, change the if-statement so it uses app?.owner_org instead of separate existence check (e.g., if (app?.owner_org !== signedScopedPath.owner_org) quickError(...)), keeping the same quickError call and behavior; ensure you reference getAppByAppIdPg, app, signedScopedPath.owner_org, and quickError when making the change.supabase/functions/_backend/plugins/updates.ts (1)
18-23: ⚡ Quick winRun the IP guard before request body parsing
isUpdateEnumerationLimited()is body-independent, so checking it beforeparseBody()avoids extra parse/validation work for already-limited traffic on/updates.As per coding guidelines: “Plugin endpoints (`/updates`, `/stats`, `/channel_self`) are extremely hot paths...”.Proposed reorder diff
app.post('/', async (c) => { - const body = await parseBody<AppInfos>(c) - cloudlog({ requestId: c.get('requestId'), message: 'post updates body', body }) const updateEnumerationLimit = await isUpdateEnumerationLimited(c) if (updateEnumerationLimit.limited) { return updateEnumerationLimitedResponse(c) } + const body = await parseBody<AppInfos>(c) + cloudlog({ requestId: c.get('requestId'), message: 'post updates body', body }) if (isLimited(c, body.app_id)) { return simpleRateLimit(body) }🤖 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 `@supabase/functions/_backend/plugins/updates.ts` around lines 18 - 23, Move the update-enumeration guard ahead of body parsing so we avoid unnecessary parse/validation on hot `/updates` traffic: call isUpdateEnumerationLimited(c) and, if limited, immediately return updateEnumerationLimitedResponse(c) before invoking parseBody<AppInfos>(c) or emitting cloudlog for the body; keep existing requestId usage when checking the guard so logging/metrics remain intact in the early-return path.
🤖 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 `@supabase/functions/_backend/utils/updateOracleGuard.ts`:
- Around line 59-67: The cache read calls in isUpdateEnumerationLimited (and the
analogous cache accesses later) currently let exceptions from
buildUpdateEnumerationCacheRequest or cacheEntry.helper.matchJson bubble up and
break the /updates guard; wrap the entire cache lookup and JSON match in a
try/catch so any thrown error causes an early return of { limited: false }
(best-effort fail-open) and optionally log the error for diagnostics; apply the
same try/catch pattern to the other cache access sites referenced (the
functions/blocks using buildUpdateEnumerationCacheRequest and
cacheEntry.helper.matchJson around the other guarded checks) so cache API
failures never turn into request failures.
- Around line 110-114: The cloudlog call in updateOracleGuard.ts is logging the
raw appId (variable appId) in the reconnaissance-handling path; change the
payload so it does not include plaintext appId and instead logs appIdHash (or a
short prefix/sanitized substring of appIdHash) and remove or redact appId from
the cloudlog arguments (the cloudlog(...) invocation that currently includes
requestId, message, appId, count). Ensure you update any variable references
used in that cloudlog call (e.g., appId -> appIdHash or appIdHash.slice(0,8)) so
only non-sensitive hashed/shortened tenant identifiers are emitted.
---
Nitpick comments:
In `@supabase/functions/_backend/files/files.ts`:
- Around line 467-470: Replace the two-step null and owner_org check with a
single optional-chaining conditional to simplify the logic: in the block using
getAppByAppIdPg and quickError, change the if-statement so it uses
app?.owner_org instead of separate existence check (e.g., if (app?.owner_org !==
signedScopedPath.owner_org) quickError(...)), keeping the same quickError call
and behavior; ensure you reference getAppByAppIdPg, app,
signedScopedPath.owner_org, and quickError when making the change.
In `@supabase/functions/_backend/plugins/updates.ts`:
- Around line 18-23: Move the update-enumeration guard ahead of body parsing so
we avoid unnecessary parse/validation on hot `/updates` traffic: call
isUpdateEnumerationLimited(c) and, if limited, immediately return
updateEnumerationLimitedResponse(c) before invoking parseBody<AppInfos>(c) or
emitting cloudlog for the body; keep existing requestId usage when checking the
guard so logging/metrics remain intact in the early-return path.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9913ff26-7059-4161-9010-e58defbb4670
📒 Files selected for processing (9)
supabase/functions/_backend/files/files.tssupabase/functions/_backend/plugins/updates.tssupabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/updateOracleGuard.tstests/files-app-read-guard.unit.test.tstests/files-local-read-proxy.unit.test.tstests/files-r2-error.test.tstests/files-security.test.tstests/update-oracle-guard.unit.test.ts
![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: e326a5275e
ℹ️ 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".
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
e326a52 to
44a10f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@supabase/functions/_backend/utils/updateOracleGuard.ts`:
- Around line 101-107: The current non-atomic flow that reads with
cacheEntry.helper.matchJson<UpdateEnumerationData>(...) then computes and writes
back with putJson can lose increments under concurrency; update the logic in
buildUpdateEnumerationCacheRequest / callers (where matchJson and putJson are
used, and around UpdateEnumerationData) to perform an atomic upsert keyed by
window+IP+appHash (or use your store's transactional primitive / durable object)
so each miss increments exactly once per window; specifically replace the
matchJson→compute→putJson sequence with a single atomic operation (or a
compare-and-swap / transaction API on cacheEntry.helper) that merges increments
server-side, or implement retry CAS if your helper exposes a version/token,
ensuring uniqueness by the combined key.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe9ef3a4-4871-4a70-b172-9df3f48abcce
📒 Files selected for processing (9)
supabase/functions/_backend/files/files.tssupabase/functions/_backend/plugins/updates.tssupabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/updateOracleGuard.tstests/files-app-read-guard.unit.test.tstests/files-local-read-proxy.unit.test.tstests/files-r2-error.test.tstests/files-security.test.tstests/update-oracle-guard.unit.test.ts
✅ Files skipped from review due to trivial changes (2)
- tests/update-oracle-guard.unit.test.ts
- tests/files-security.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/files-local-read-proxy.unit.test.ts
- supabase/functions/_backend/plugins/updates.ts
- tests/files-app-read-guard.unit.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44a10f40ba
ℹ️ 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".
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
44a10f4 to
b30b6e8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b30b6e80c3
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
supabase/functions/_backend/utils/updateOracleGuard.ts (1)
127-132:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap limited-marker cache writes to keep the guard fail-open.
putJson()at Line 128 can still throw and fail/updateseven after computinglimited. This should follow the same best-effort error handling used elsewhere.Suggested hardening
if (limited) { - await cacheEntry.helper.putJson( - buildUpdateEnumerationLimitRequest(cacheEntry.helper, cacheEntry.ip), - { resetAt: bucketState.resetAt ?? resetAt }, - UPDATE_ENUMERATION_TTL_SECONDS, - ) + try { + await cacheEntry.helper.putJson( + buildUpdateEnumerationLimitRequest(cacheEntry.helper, cacheEntry.ip), + { resetAt: bucketState.resetAt ?? resetAt }, + UPDATE_ENUMERATION_TTL_SECONDS, + ) + } + catch (error) { + cloudlog({ + requestId: c.get('requestId'), + message: 'Update enumeration guard limit write failed', + error, + }) + } }🤖 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 `@supabase/functions/_backend/utils/updateOracleGuard.ts` around lines 127 - 132, The write of the "limited" marker using cacheEntry.helper.putJson (with buildUpdateEnumerationLimitRequest and UPDATE_ENUMERATION_TTL_SECONDS) can throw and must be made best-effort so the guard stays fail-open; wrap the await cacheEntry.helper.putJson(...) call in a try/catch around the limited branch, catch and swallow/log the error (using the existing logger or cacheEntry.helper logger) without rethrowing so failures to write the cache do not fail the /updates path.
🤖 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 `@supabase/functions/_backend/files/files.ts`:
- Around line 479-482: The current early return that allows
isAttachmentExistenceProbe to bypass auth (the if block checking
!isUploadAttachmentRoute(c) && isAttachmentExistenceProbe(c)) must be removed so
unauthenticated callers cannot infer existence or fetch ranges; instead, enforce
the same signed-read/auth gate used for normal attachment fetches by removing
the bypass and routing such probes through the existing signed-read verification
path (i.e., have attachment probes pass through the same authentication check
used by the attachment GET handler), ensuring functions like
isAttachmentExistenceProbe and isUploadAttachmentRoute no longer allow
unauthenticated short-circuits.
In `@supabase/functions/_backend/utils/update.ts`:
- Around line 131-134: The code calls setAppStatus even for requests already
marked limited by recordUpdateEnumerationMiss; move the early return so you
check updateEnumerationLimit.limited immediately after calling
recordUpdateEnumerationMiss and return updateEnumerationLimitedResponse(c)
before calling setAppStatus(c, app_id, 'onprem', true). Update the flow in the
same function to call recordUpdateEnumerationMiss(...), if limited -> return
updateEnumerationLimitedResponse(...); otherwise proceed to setAppStatus(...)
and continue.
In `@supabase/functions/_backend/utils/updateOracleGuard.ts`:
- Around line 37-40: The current getUpdateEnumerationBucket logic (and the
similar checks at the other spots referenced) is vulnerable because it
deterministically maps app IDs to buckets and counts occupied buckets rather
than distinct app IDs; to fix it, stop relying on plain sha256(appId) % N as the
anti-enumeration signal and instead (1) track uniqueness by hashed app ID
(store/compare H(appId) or HMAC(secret, appId)) when counting updates so you
count distinct app IDs, and (2) make the bucket selection unpredictable to
attackers by using a server-side secret (e.g., HMAC with a rotating secret) when
computing the bucket in getUpdateEnumerationBucket so precomputation is
infeasible; update all corresponding guard checks (the other blocks at 64-67 and
111-113) to use the new distinct-hash tracking and keyed bucket mapping.
---
Duplicate comments:
In `@supabase/functions/_backend/utils/updateOracleGuard.ts`:
- Around line 127-132: The write of the "limited" marker using
cacheEntry.helper.putJson (with buildUpdateEnumerationLimitRequest and
UPDATE_ENUMERATION_TTL_SECONDS) can throw and must be made best-effort so the
guard stays fail-open; wrap the await cacheEntry.helper.putJson(...) call in a
try/catch around the limited branch, catch and swallow/log the error (using the
existing logger or cacheEntry.helper logger) without rethrowing so failures to
write the cache do not fail the /updates path.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05c52510-46b4-4c68-8572-f31df0ca4f26
📒 Files selected for processing (8)
supabase/functions/_backend/files/files.tssupabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/updateOracleGuard.tstests/files-app-read-guard.unit.test.tstests/files-local-read-proxy.unit.test.tstests/files-r2-error.test.tstests/files-security.test.tstests/update-oracle-guard.unit.test.ts
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
b30b6e8 to
89bbbdb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89bbbdb212
ℹ️ 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".
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
89bbbdb to
2cee9e7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cee9e732c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 269fb5a50b
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
supabase/functions/_backend/utils/updateOracleGuard.ts (1)
194-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the limit-marker write fail-open.
Line 195 is the one remaining cache write outside the guarded best-effort path. If
putJson()throws after the threshold is reached,recordUpdateEnumerationMiss()will bubble an exception and/updatesreturns a 500 instead of a limited response.Suggested hardening
if (limited) { - await cacheEntry.helper.putJson( - buildUpdateEnumerationLimitRequest(cacheEntry.helper, cacheEntry.ip), - { resetAt: missState.resetAt ?? resetAt }, - UPDATE_ENUMERATION_TTL_SECONDS, - ) + try { + await cacheEntry.helper.putJson( + buildUpdateEnumerationLimitRequest(cacheEntry.helper, cacheEntry.ip), + { resetAt: missState.resetAt ?? resetAt }, + UPDATE_ENUMERATION_TTL_SECONDS, + ) + } + catch (error) { + cloudlog({ + requestId: c.get('requestId'), + message: 'Update enumeration guard limit write failed', + error, + }) + } }🤖 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 `@supabase/functions/_backend/utils/updateOracleGuard.ts` around lines 194 - 199, The cache write at cacheEntry.helper.putJson (invoked with buildUpdateEnumerationLimitRequest and resetAt) must be made fail-open: wrap the await cacheEntry.helper.putJson(...) call in a try/catch so any thrown error is swallowed or logged but not re-thrown, ensuring recordUpdateEnumerationMiss()/the /updates response does not bubble a 500; update the code in updateOracleGuard.ts around the limited branch to catch exceptions from putJson and proceed as a best-effort operation.
🧹 Nitpick comments (1)
tests/update-oracle-guard.unit.test.ts (1)
77-79: ⚡ Quick winAssert the limited payload exactly.
toMatchObject({ error: 'on_premise_app' })still passes when extra keys are added, so these tests would miss a response-shape regression on the cache-sensitive 429 contract. An exact equality assertion would lock that down.Suggested assertion change
- await expect(blocked.json()).resolves.toMatchObject({ error: 'on_premise_app' }) + await expect(blocked.json()).resolves.toEqual({ error: 'on_premise_app' })- await expect(blocked.json()).resolves.toMatchObject({ error: 'on_premise_app' }) + await expect(blocked.json()).resolves.toEqual({ error: 'on_premise_app' })As per coding guidelines, "Maintain cache-specific response codes and body shapes for plugin endpoints (
/updates,/stats,/channel_self): return 429 + {error: 'on_premise_app'} or {error: 'need_plan_upgrade'} to trigger Cloudflare snippet caching; do not change these without updating the Cloudflare snippet and app status logic".Also applies to: 130-131
🤖 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 `@tests/update-oracle-guard.unit.test.ts` around lines 77 - 79, The test currently uses a loose shape check (await expect(blocked.json()).resolves.toMatchObject({ error: 'on_premise_app' })) which allows extra keys; change it to an exact equality assertion such as await expect(blocked.json()).resolves.toEqual({ error: 'on_premise_app' }) or toStrictEqual to lock the 429 response body shape, and apply the same change to the other occurrence mentioned (the assertion around lines 130-131) so the cache-sensitive contract is enforced; locate the assertions using the blocked variable and the blocked.json() call in update-oracle-guard.unit.test.ts to update them.
🤖 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 `@supabase/functions/_backend/utils/updateOracleGuard.ts`:
- Around line 215-216: The function updateEnumerationLimitedResponse currently
returns a body with { error: 'on_premise_app', message: 'On-premise app
detected' } which breaks the cache-compatible shape; change the response to
return only { error: 'on_premise_app' } with the same 429 status code (i.e.,
update the c.json(...) call in updateEnumerationLimitedResponse to emit only the
error property) so the /updates cache handling and Cloudflare snippet continue
to work as expected.
---
Duplicate comments:
In `@supabase/functions/_backend/utils/updateOracleGuard.ts`:
- Around line 194-199: The cache write at cacheEntry.helper.putJson (invoked
with buildUpdateEnumerationLimitRequest and resetAt) must be made fail-open:
wrap the await cacheEntry.helper.putJson(...) call in a try/catch so any thrown
error is swallowed or logged but not re-thrown, ensuring
recordUpdateEnumerationMiss()/the /updates response does not bubble a 500;
update the code in updateOracleGuard.ts around the limited branch to catch
exceptions from putJson and proceed as a best-effort operation.
---
Nitpick comments:
In `@tests/update-oracle-guard.unit.test.ts`:
- Around line 77-79: The test currently uses a loose shape check (await
expect(blocked.json()).resolves.toMatchObject({ error: 'on_premise_app' }))
which allows extra keys; change it to an exact equality assertion such as await
expect(blocked.json()).resolves.toEqual({ error: 'on_premise_app' }) or
toStrictEqual to lock the 429 response body shape, and apply the same change to
the other occurrence mentioned (the assertion around lines 130-131) so the
cache-sensitive contract is enforced; locate the assertions using the blocked
variable and the blocked.json() call in update-oracle-guard.unit.test.ts to
update them.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12900c53-26a5-41fb-9445-6753def77943
📒 Files selected for processing (6)
cli/src/bundle/upload.tscli/src/bundle/zip.tscli/test/fixtures/setup-test-projects.shsupabase/functions/_backend/utils/update.tssupabase/functions/_backend/utils/updateOracleGuard.tstests/update-oracle-guard.unit.test.ts
✅ Files skipped from review due to trivial changes (1)
- cli/src/bundle/upload.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61986d9b82
ℹ️ 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".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?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: 357f11fb3d
ℹ️ 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".
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
28233e0 to
032c01b
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
riderx
force-pushed
the
codex/fix-update-enumeration-oracle
branch
from
4cbcb48 to
3dd9714
Compare
|
1 participant
Summary (AI generated)
/updatesapp-id misses.Motivation (AI generated)
GHSA-4pq5-xr4m-6gxx reported that
/updatescould be used as an unauthenticated app-id oracle and that attachment reads were not validating caller access or the update-issued download key.Business Impact (AI generated)
This reduces cross-tenant bundle exposure risk and limits unauthenticated customer/app reconnaissance while keeping existing updater download URLs compatible.
Test Plan (AI generated)
bun lintbun lint:backendbunx eslint tests/update-oracle-guard.unit.test.ts tests/files-app-read-guard.unit.test.ts tests/files-security.test.tsbunx vitest run tests/update-oracle-guard.unit.test.ts tests/files-app-read-guard.unit.test.tsbun run supabase:with-env -- bunx vitest run tests/updates.test.ts tests/files-security.test.tsbun typecheckGenerated with AI
Summary by CodeRabbit
New Features
Tests
Chores