Fix repository regression findings#912
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34939609 | Triggered | PostgreSQL Credentials | f27b9ef | scripts/verify-release-offline.mjs | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR adds route-specific therapy catalogue loading and generated-index validation, propagates abort signals through search and RAG workflows, preserves semantic reranking signals, tightens API/auth validation, persists factsheet state, strengthens PDF extraction budgets, and updates CI/offline verification. ChangesCore application hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
@codex please fix this issue for me and remove the secrete from being accessible here |
![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: f27b9ef4e8
ℹ️ 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".
| if (userResult.error && !isDefinitiveAuthValidationError(userResult.error)) { | ||
| setSession(null); | ||
| setStatus("error"); | ||
| setNotice(null); | ||
| setError("Session could not be verified. Check your connection and retry."); | ||
| return; |
There was a problem hiding this comment.
When getUser() fails because Supabase Auth is temporarily unreachable (for example a retryable fetch/503 error) while getSession() still returns a stored session, this guard returns before the existing verificationUnavailable path can preserve that session. That changes transient offline/auth-service outages from “keep the signed-in UI; server routes still validate tokens” into status: "error" with session cleared, leaving valid users unable to use the app until auth verification succeeds again.
Useful? React with 👍 / 👎.
|
Summary
Testing
|
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
|
Superseded by PR #913 after tracing GitGuardian's finding to an intentionally invalid offline PostgreSQL placeholder and removing the credential-shaped literal from the clean one-commit history. |
Pull request was closed
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f27b9ef4e8
ℹ️ 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".
| if (userResult.error && !isDefinitiveAuthValidationError(userResult.error)) { | ||
| setSession(null); | ||
| setStatus("error"); | ||
| setNotice(null); | ||
| setError("Session could not be verified. Check your connection and retry."); | ||
| return; |
There was a problem hiding this comment.
When getUser() fails for a retryable network/auth-service outage but getSession() still returns a valid stored session, this early return sets session to null and leaves the app in error, so the verificationUnavailable path below never runs and logged-in users lose authenticated UI/API headers until a reload succeeds. This regresses the documented fallback in resolveInitialAuthState; let non-definitive errors continue to that resolver (or only show the error when there is no stored session) so transient outages do not break signed-in access.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/rag.ts (1)
3303-3313: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftKeep shared work alive when the originator disconnects.
The pending producer is created with the originating caller’s
args.signal. If that caller aborts, remaining waiters hit this catch and start a replacement run, duplicating retrieval/generation instead of retaining shared work. Store a shared controller and waiter count with the in-flight entry; pass its signal to the producer and abort it only after the last caller leaves. Add the inverse test: abort the originator while a waiter remains.As per coding guidelines, “For clinical, retrieval, answer-generation… changes, verify realistic failure paths and preserve conservative fallback behavior; add focused tests when needed.”
🤖 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/lib/rag.ts` around lines 3303 - 3313, Update the in-flight entry and producer flow around awaitWithCallerSignal so shared work uses a dedicated controller signal rather than the originating caller’s args.signal. Track waiter count for each in-flight entry, decrement it when callers leave, and abort the shared controller only after the final caller exits; preserve each caller’s own cancellation behavior. Add a focused test covering originator cancellation while another waiter remains, verifying the existing shared work completes without starting a replacement run.Source: Coding guidelines
🧹 Nitpick comments (2)
tests/offline-release-profile.test.ts (1)
28-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the complete offline-profile contract.
The runner adds three new check commands and two cross-tenant placeholders, but this test asserts only
eval:rag:offlineand four environment values. Add exact assertions forcheck:function-grants,check:owner-scope,check:rag:fixtures,CROSS_TENANT_SUPABASE_URL, andCROSS_TENANT_SUPABASE_SERVICE_ROLE_KEY.🤖 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/offline-release-profile.test.ts` around lines 28 - 35, Extend the offline release profile assertions in the releaseRunner test to cover the complete contract: add exact presence checks for "check:function-grants", "check:owner-scope", and "check:rag:fixtures", plus the CROSS_TENANT_SUPABASE_URL and CROSS_TENANT_SUPABASE_SERVICE_ROLE_KEY placeholder values. Preserve the existing assertions.tests/private-client-auth.test.ts (1)
44-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the AuthProvider initialization behavior, not only the classifier.
These assertions pass with both the current inverted guard and the corrected guard. Add a regression case with a valid stored session and a transient
getUser()failure (503or fetch error), asserting that the session is preserved; also cover definitive rejection clearing it.🤖 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/private-client-auth.test.ts` around lines 44 - 50, Extend the AuthProvider initialization tests beyond isDefinitiveAuthValidationError: add a valid stored-session case where transient getUser() failure (503 or fetch error) preserves the session, and a definitive rejection case where getUser() failure clears it. Exercise the actual initialization behavior and assert the resulting session state, using the existing AuthProvider setup and storage helpers.
🤖 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/api/documents/`[id]/summarize/route.ts:
- Line 42: Update the summarizeDocument flow called by the route to propagate
request.signal through document and chunk retrieval, including the unindexed
fallback computation. Ensure each awaited read observes the signal or checks for
abortion immediately afterward so disconnected requests stop further work, while
preserving normal summary generation behavior.
In `@src/app/api/search/route.ts`:
- Around line 124-153: Evict inflight entries when the final waiter aborts
unsettled shared work. In src/app/api/search/route.ts:124-153, update
coalesceScopedSearch so the last-waiter abort also conditionally deletes the
matching scopedSearchInflight entry. In src/lib/openai.ts:666-674, pass the
cache key from embedTextWithTelemetry into awaitInflightEmbedding and
conditionally delete the matching queryEmbeddingInflight entry alongside the
abort.
In `@src/lib/extractors/document.ts`:
- Around line 243-252: Update the PDFParse construction in the document
extraction flow to pass maxImageSize: limits.maxRenderPixels, ensuring the
parser enforces the render-pixel cap during image decoding. Keep the existing
budget.assertRenderDimensions check in the image-processing loop unchanged as a
post-decode validation.
In `@src/lib/supabase/client.tsx`:
- Around line 182-188: Invert the condition in the user session verification
flow so the early session clearing, error status, and return execute only when
isDefinitiveAuthValidationError(userResult.error) is true. Preserve transient
503 and fetch-error handling so verificationUnavailable can reach the resolver
without clearing the stored session.
---
Outside diff comments:
In `@src/lib/rag.ts`:
- Around line 3303-3313: Update the in-flight entry and producer flow around
awaitWithCallerSignal so shared work uses a dedicated controller signal rather
than the originating caller’s args.signal. Track waiter count for each in-flight
entry, decrement it when callers leave, and abort the shared controller only
after the final caller exits; preserve each caller’s own cancellation behavior.
Add a focused test covering originator cancellation while another waiter
remains, verifying the existing shared work completes without starting a
replacement run.
---
Nitpick comments:
In `@tests/offline-release-profile.test.ts`:
- Around line 28-35: Extend the offline release profile assertions in the
releaseRunner test to cover the complete contract: add exact presence checks for
"check:function-grants", "check:owner-scope", and "check:rag:fixtures", plus the
CROSS_TENANT_SUPABASE_URL and CROSS_TENANT_SUPABASE_SERVICE_ROLE_KEY placeholder
values. Preserve the existing assertions.
In `@tests/private-client-auth.test.ts`:
- Around line 44-50: Extend the AuthProvider initialization tests beyond
isDefinitiveAuthValidationError: add a valid stored-session case where transient
getUser() failure (503 or fetch error) preserves the session, and a definitive
rejection case where getUser() failure clears it. Exercise the actual
initialization behavior and assert the resulting session state, using the
existing AuthProvider setup and storage helpers.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 825e94bb-5e95-4c98-9317-b91e7acbca8e
📒 Files selected for processing (50)
.github/workflows/ci.ymldocs/branch-review-ledger.mdpackage.jsonpublic/therapy-compass-data/therapies-index.jsonscripts/build-therapies-index.mjsscripts/ci-change-scope.mjsscripts/verify-release-offline.mjssrc/app/api/documents/[id]/route.tssrc/app/api/documents/[id]/summarize/route.tssrc/app/api/search/route.tssrc/components/factsheets/factsheet-detail-page.tsxsrc/components/factsheets/factsheets-data.tssrc/components/therapy-compass/bindings.tsxsrc/components/therapy-compass/data/use-therapy-data.tssrc/components/therapy-compass/screens/brief-screen.tsxsrc/components/therapy-compass/screens/pathways-screen.tsxsrc/components/therapy-compass/screens/recommend-screen.tsxsrc/components/therapy-compass/therapy-card.tsxsrc/data/therapies-index.jsonsrc/lib/answer-ranking.tssrc/lib/document-detail.tssrc/lib/document-enrichment.tssrc/lib/extractors/document.tssrc/lib/extractors/pdf-extraction-budget.tssrc/lib/openai.tssrc/lib/rag-versioning.tssrc/lib/rag.tssrc/lib/saved-registry-storage.tssrc/lib/search-scope.tssrc/lib/semantic-rerank.tssrc/lib/supabase/client.tsxsrc/lib/types.tstests/answer-ranking.test.tstests/api-validation-contract.test.tstests/embed-texts-integrity.test.tstests/factsheet-save.dom.test.tsxtests/factsheets-data.test.tstests/offline-release-profile.test.tstests/openai-cache.test.tstests/pdf-extraction-budget.test.tstests/pdf-extractor.test.tstests/private-access-routes.test.tstests/private-client-auth.test.tstests/rag-answer-fallback.test.tstests/rag-document-summary.test.tstests/semantic-rerank.test.tstests/therapy-compass-artifact-navigation.dom.test.tsxtests/therapy-compass-data-recovery.dom.test.tsxtests/therapy-compass-mode-wiring.test.tstests/ui-route-coverage.spec.ts
| if (rateLimit.limited) | ||
| return rateLimitJsonResponse("Too many document summary requests. Retry shortly.", rateLimit); | ||
| const answer = await summarizeDocument(id, user.id); | ||
| const answer = await summarizeDocument(id, user.id, { signal: request.signal }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== route.ts ==\n'
cat -n src/app/api/documents/[id]/summarize/route.ts
printf '\n== summarizeDocument references ==\n'
rg -n "function summarizeDocument|const summarizeDocument|export .*summarizeDocument|summarizeDocument\(" src -S
printf '\n== candidate implementation ==\n'
fd -a "summarizeDocument.*" srcRepository: BigSimmo/Database
Length of output: 3839
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== summarizeDocument implementation ==\n'
sed -n '5040,5145p' src/lib/rag.ts
printf '\n== stream route usage ==\n'
sed -n '200,260p' src/app/api/answer/stream/route.tsRepository: BigSimmo/Database
Length of output: 7978
Thread the abort signal through summary retrieval.
request.signal only reaches structured generation here; the document/chunk reads still run after disconnect, and the unindexed fallback can still be computed. Pass the signal into those reads or bail out after each await so aborted requests stop work earlier.
🤖 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/api/documents/`[id]/summarize/route.ts at line 42, Update the
summarizeDocument flow called by the route to propagate request.signal through
document and chunk retrieval, including the unindexed fallback computation.
Ensure each awaited read observes the signal or checks for abortion immediately
afterward so disconnected requests stop further work, while preserving normal
summary generation behavior.
Source: Coding guidelines
| async function coalesceScopedSearch<T extends Record<string, unknown>>( | ||
| key: string, | ||
| producer: (signal: AbortSignal) => Promise<T>, | ||
| signal: AbortSignal, | ||
| ) { | ||
| signal.throwIfAborted(); | ||
| let entry = scopedSearchInflight.get(key); | ||
| const coalesced = Boolean(entry); | ||
| if (!entry) { | ||
| const controller = new AbortController(); | ||
| const created: ScopedSearchInflight = { | ||
| promise: Promise.resolve({}), | ||
| controller, | ||
| waiters: 0, | ||
| settled: false, | ||
| }; | ||
| created.promise = producer(controller.signal).finally(() => { | ||
| created.settled = true; | ||
| if (scopedSearchInflight.get(key) === created) scopedSearchInflight.delete(key); | ||
| }); | ||
| scopedSearchInflight.set(key, created); | ||
| entry = created; | ||
| } | ||
| entry.waiters += 1; | ||
| try { | ||
| return { payload: (await awaitWithCallerSignal(entry.promise, signal)) as T, coalesced }; | ||
| } finally { | ||
| entry.waiters -= 1; | ||
| if (entry.waiters === 0 && !entry.settled) entry.controller.abort(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Evict the inflight entry when the last waiter aborts the shared producer (both coalescers). In both files, when waiters reaches 0 and work is unsettled you abort the shared controller, but the map entry is only removed in the producer's own .finally, which runs after the producer rejects. A fresh caller with the same key/text arriving in that window coalesces onto the aborting producer and receives a spurious abort rejection despite never cancelling. Evict the entry at the same point you trigger the abort.
src/app/api/search/route.ts#L124-L153: in thefinally, whenentry.waiters === 0 && !entry.settled, afterentry.controller.abort()also doif (scopedSearchInflight.get(key) === entry) scopedSearchInflight.delete(key)(thekeyis already in scope here).src/lib/openai.ts#L666-L674: thread the cachekeyfromembedTextWithTelemetryintoawaitInflightEmbeddingand, on the last-waiter abort,if (queryEmbeddingInflight.get(key) === entry) queryEmbeddingInflight.delete(key)alongsideentry.controller.abort().
📍 Affects 2 files
src/app/api/search/route.ts#L124-L153(this comment)src/lib/openai.ts#L666-L674
🤖 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/api/search/route.ts` around lines 124 - 153, Evict inflight entries
when the final waiter aborts unsettled shared work. In
src/app/api/search/route.ts:124-153, update coalesceScopedSearch so the
last-waiter abort also conditionally deletes the matching scopedSearchInflight
entry. In src/lib/openai.ts:666-674, pass the cache key from
embedTextWithTelemetry into awaitInflightEmbedding and conditionally delete the
matching queryEmbeddingInflight entry alongside the abort.
| const imageResult = await parser.getImage({ | ||
| partial: [rawPage.pageNumber], | ||
| imageBuffer: true, | ||
| imageDataUrl: false, | ||
| imageThreshold: 20, | ||
| }); | ||
| for (const page of imageResult.pages) { | ||
| for (const [index, image] of page.images.entries()) { | ||
| budget.assertRenderDimensions(image.width, image.height); | ||
| budget.assertArtifact(image.data.byteLength); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'new PDFParse|\.getImage\(|assertRenderDimensions|maxImageSize' \
src/lib/extractors/document.ts \
src/lib/extractors/pdf-extraction-budget.ts
if [ -f node_modules/pdf-parse/dist/esm/LoadParameters.d.ts ]; then
rg -n -C2 'maxImageSize' node_modules/pdf-parse/dist/esm/LoadParameters.d.ts
fiRepository: BigSimmo/Database
Length of output: 2663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== document.ts context =="
sed -n '218,258p' src/lib/extractors/document.ts
echo
echo "== pdf-extraction budget =="
sed -n '1,140p' src/lib/extractors/pdf-extraction-budget.ts
echo
echo "== pdf-parse package files =="
fd -a 'pdf-parse' node_modules | head -n 50 || true
echo
echo "== search for maxImageSize in pdf-parse =="
rg -n -C2 'maxImageSize|imageThreshold|getImage\(|class PDFParse|constructor\(' node_modules/pdf-parse -g '!**/*.map' || trueRepository: BigSimmo/Database
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
set -o pipefail
python3 - <<'PY'
import json, os, pathlib, re, sys
root = pathlib.Path("node_modules/pdf-parse")
candidates = []
for p in root.rglob("*"):
if p.is_file() and p.suffix in {".js", ".d.ts", ".md", ".ts"}:
text = p.read_text(errors="ignore")
if "maxImageSize" in text or "imageThreshold" in text or "getImage(" in text or "class PDFParse" in text:
candidates.append(str(p))
print("\n".join(candidates[:100]))
PYRepository: BigSimmo/Database
Length of output: 735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== document.ts excerpt =="
sed -n '220,256p' src/lib/extractors/document.ts
echo
echo "== PDFParse constructor type =="
sed -n '1,220p' node_modules/pdf-parse/dist/pdf-parse/esm/PDFParse.d.ts
echo
echo "== load parameters type =="
sed -n '1,220p' node_modules/pdf-parse/dist/pdf-parse/esm/LoadParameters.d.ts
echo
echo "== parse parameters type =="
sed -n '1,220p' node_modules/pdf-parse/dist/pdf-parse/esm/ParseParameters.d.ts
echo
echo "== README mentions of maxImageSize =="
rg -n -C2 'maxImageSize|imageThreshold|maxRenderPixels|LoadParameters|PDFParse' node_modules/pdf-parse/README.mdRepository: BigSimmo/Database
Length of output: 23578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in [
Path("node_modules/pdf-parse/dist/pdf-parse/esm/PDFParse.d.ts"),
Path("node_modules/pdf-parse/dist/pdf-parse/esm/LoadParameters.d.ts"),
Path("node_modules/pdf-parse/dist/pdf-parse/esm/ParseParameters.d.ts"),
]:
print(f"\n== {path} ==")
text = path.read_text(errors="ignore").splitlines()
for i, line in enumerate(text[:220], 1):
if "maxImageSize" in line or "imageThreshold" in line or "constructor" in line or "getImage" in line or "interface" in line or "type" in line:
print(f"{i}:{line}")
PYRepository: BigSimmo/Database
Length of output: 1710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== document.ts excerpt =="
sed -n '220,256p' src/lib/extractors/document.ts
echo
echo "== PDFParse.d.ts =="
cat -n node_modules/pdf-parse/dist/pdf-parse/esm/PDFParse.d.ts | sed -n '1,220p'
echo
echo "== LoadParameters.d.ts =="
cat -n node_modules/pdf-parse/dist/pdf-parse/esm/LoadParameters.d.ts | sed -n '1,220p'
echo
echo "== ParseParameters.d.ts =="
cat -n node_modules/pdf-parse/dist/pdf-parse/esm/ParseParameters.d.ts | sed -n '1,220p'
echo
echo "== README hits =="
rg -n -C2 'maxImageSize|imageThreshold|maxRenderPixels|LoadParameters|PDFParse' node_modules/pdf-parse/README.mdRepository: BigSimmo/Database
Length of output: 26582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [
Path("node_modules/pdf-parse/dist/pdf-parse/esm/PDFParse.d.ts"),
Path("node_modules/pdf-parse/dist/pdf-parse/esm/LoadParameters.d.ts"),
Path("node_modules/pdf-parse/dist/pdf-parse/esm/ParseParameters.d.ts"),
]:
print(f"\n== {p} ==")
for i, line in enumerate(p.read_text(errors="ignore").splitlines(), 1):
if i <= 220 and any(k in line for k in ("maxImageSize", "imageThreshold", "constructor", "getImage", "interface", "type ", "class PDFParse")):
print(f"{i}:{line}")
PYRepository: BigSimmo/Database
Length of output: 1668
Pass the parser a decode-time image cap. budget.assertRenderDimensions() only runs after getImage() has already allocated the bitmap, so large images can still spike memory here. Set maxImageSize: limits.maxRenderPixels on new PDFParse(...) so the limit applies before decoding.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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/lib/extractors/document.ts` around lines 243 - 252, Update the PDFParse
construction in the document extraction flow to pass maxImageSize:
limits.maxRenderPixels, ensuring the parser enforces the render-pixel cap during
image decoding. Keep the existing budget.assertRenderDimensions check in the
image-processing loop unchanged as a post-decode validation.
Source: Coding guidelines
| if (userResult.error && !isDefinitiveAuthValidationError(userResult.error)) { | ||
| setSession(null); | ||
| setStatus("error"); | ||
| setNotice(null); | ||
| setError("Session could not be verified. Check your connection and retry."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Only clear the session for definitive auth rejection.
Lines 182-188 currently clear the stored session when isDefinitiveAuthValidationError(...) is false. That includes the transient 503 and fetch-error cases covered by tests/private-client-auth.test.ts, so a temporary auth outage signs out otherwise valid users and prevents verificationUnavailable from reaching the resolver.
Invert the predicate so only definitive token/session rejection takes this early-clear path.
Suggested correction
- if (userResult.error && !isDefinitiveAuthValidationError(userResult.error)) {
+ if (userResult.error && isDefinitiveAuthValidationError(userResult.error)) {📝 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.
| if (userResult.error && !isDefinitiveAuthValidationError(userResult.error)) { | |
| setSession(null); | |
| setStatus("error"); | |
| setNotice(null); | |
| setError("Session could not be verified. Check your connection and retry."); | |
| return; | |
| } | |
| if (userResult.error && isDefinitiveAuthValidationError(userResult.error)) { | |
| setSession(null); | |
| setStatus("error"); | |
| setNotice(null); | |
| setError("Session could not be verified. Check your connection and retry."); | |
| return; | |
| } |
🤖 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/lib/supabase/client.tsx` around lines 182 - 188, Invert the condition in
the user session verification flow so the early session clearing, error status,
and return execute only when isDefinitiveAuthValidationError(userResult.error)
is true. Preserve transient 503 and fetch-error handling so
verificationUnavailable can reach the resolver without clearing the stored
session.
1 participant
Summary
Verification
Verification not run: provider-backed OpenAI retrieval/answer evaluations were not run because live provider access was outside this remediation workflow.
Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
New Features
Bug Fixes