Update idb-keyval version in package.json#214
Conversation
Added common issues and suggested improvements for the guide.
This document provides common issues and suggestions for improving user experience with ExtensionShield, including environment configuration, URL requirements, analysis results, setup overview, and CLI usage clarity.
Removed the 'Common Issues & Improvements' section from the GET_STARTED.md file.
Removed example for CLI usage to streamline documentation.
Added section for additional resources and common issues.
|
Hi @sapnilbiswas, |
|
@Stanzin7 can you please have a look at this |
|
Hi @sapnilbiswas, The CI issue has been resolved. The failure was caused by an outdated branch state, resulting in a JSON parsing error during dependency installation. I have synced the branch with the latest master and ensured all package files are consistent. The build is now passing successfully. |
|
@Stanzin7 please have a look at this |
21c088c to
c8d3c84
Compare
- Replace strict URL validation with extractExtensionId() - Support new Chrome Web Store URLs - Fix UI showing N/A scores in partial reports - Improve scan UX and error handling
7eba044 to
dbce4a2
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdates add frontend handling for partial/incomplete scan reports (UI adjustments and navigation disabling), change scanner URL validation to extract extension IDs, bump a frontend dependency and reformat package JSON, and hardcode local on-disk paths for backend database and results storage. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/pages/reports/ReportDetailPage.jsx`:
- Around line 39-45: The isPartial calculation currently misses validating
scorecard.score so reports with missing/non-numeric scores render as 0/100;
update the isPartial logic (the isPartial constant) to treat a missing or
non-numeric report?.scorecard?.score as partial (e.g., check typeof score ===
"number" and isFinite(score) or Number.isFinite), and change any rendering that
currently falls back to 0 to show an explicit "N/A" or incomplete state when
isPartial is true (also apply the same validation where score is used later in
the score rendering block referenced near the second occurrence to avoid falling
back to 0).
- Line 47: The local state variable mode (const [mode, setMode]) is updated by
the Simple/Advanced buttons but never read in ReportViewModelDetail, so the
toggle is a no-op; fix by either removing the unused state and the toggle
buttons or wiring the state through to where it matters: pass mode as a prop
into ReportViewModelDetail (or its child components) and use it to conditionally
render the simple vs advanced UI, or remove the setMode calls and delete the
toggle markup/buttons entirely; update any handlers tied to setMode accordingly
so there are no unused state warnings.
In `@frontend/src/pages/scanner/ScannerPage.jsx`:
- Around line 405-424: The handleScanClick callback currently only validates url
and extensionId and therefore can be invoked via Enter even when the UI button
is disabled; update handleScanClick to mirror the button guards by
early-returning if isScanning is true or the daily-limit gate is reached (e.g.,
hasReachedDailyLimit or canScanToday is false) before doing any validation or
calling startScan; keep existing url/extensionId validation and setError
behavior, and ensure you still call startScan(extensionId) only after these
guard checks pass (use the same boolean flags/props used to disable the button
so keyboard/Enter respects the same limits).
- Around line 413-423: The code currently calls startScan(extensionId) but
startScan in ScanContext expects a full Chrome Web Store URL and will internally
call extractExtensionId(), so replace the argument with the original user input:
call startScan(input) after validating extensionId; keep the existing
setError(null) and return flow using extensionId only for validation, i.e., use
extensionId from realScanService.extractExtensionId(input) to validate and then
invoke startScan(input) so extractExtensionId() inside startScan receives the
full URL.
In `@frontend/src/pages/scanner/ScanResultsPageV2.jsx`:
- Around line 486-490: The partial-report detection in ScanResultsPageV2.jsx
(variable isPartialReport) is too narrow compared to ReportDetailPage and must
be replaced with the shared predicate used there; extract or import a helper
(e.g., isPartialScanReport) that checks status === "failed", error includes
"download", scorecard.confidence === "LOW", and missing/placeholder meta fields
(meta.name, "Unknown Extension", meta.publisher "Unknown"), then use that helper
wherever isPartialReport is computed (the existing isPartialReport declaration
and the other occurrences noted around the later blocks) so both pages use the
identical logic.
- Around line 379-382: The hasAnyScore boolean currently treats 0 as falsy;
change its checks to explicitly detect presence instead of truthiness by testing
for null/undefined or numeric type. Replace the expression assigning hasAnyScore
(which references scores?.security?.score, scores?.privacy?.score,
scores?.governance?.score) with something like explicit existence checks (e.g.,
scores?.security?.score != null || scores?.privacy?.score != null ||
scores?.governance?.score != null) or Number.isFinite(...) / typeof === 'number'
checks so that 0 counts as a real score; make the same change for the other
analogous block that uses these same score properties.
In `@src/extension_shield/api/database.py`:
- Line 79: Replace the hard-coded Path.cwd() usage so the code uses the
configured setting: read get_settings().database_path and use it as the default
base for db_path (falling back to Path.cwd()/data/db.sqlite3 only if
database_path is empty), updating the assignment where db_path is set in
src/extension_shield/api/database.py (the variable named db_path) so
environment/config overrides are respected.
In `@src/extension_shield/api/main.py`:
- Around line 1019-1021: Replace the hardcoded RESULTS_DIR that uses Path.cwd()
with the configured storage root from get_settings(): call get_settings(), read
the configured storage path (e.g., settings.storage_path or settings.data_dir)
and build RESULTS_DIR = Path(settings.<storage_attr>) / "extensions_storage",
then ensure RESULTS_DIR.mkdir(parents=True, exist_ok=True); update the existing
RESULTS_DIR definition and mkdir call accordingly so scan artifacts use the
configured storage instead of CWD.
🪄 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 Plus
Run ID: 8e40cac6-d25e-4967-987a-28a8374d1a83
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
frontend/package.jsonfrontend/src/pages/reports/ReportDetailPage.jsxfrontend/src/pages/scanner/ScanResultsPageV2.jsxfrontend/src/pages/scanner/ScannerPage.jsxsrc/extension_shield/api/database.pysrc/extension_shield/api/main.py
| const isPartial = | ||
| !report || | ||
| !report?.scorecard || | ||
| report?.scorecard?.confidence === "LOW" || | ||
| !report?.meta?.name || | ||
| report?.meta?.name === "Unknown Extension" || | ||
| report?.meta?.publisher === "Unknown"; |
There was a problem hiding this comment.
Don't render a fake 0/100 when the numeric score is missing.
isPartial doesn't check scorecard.score, but the score block falls back to 0. A payload with a missing / non-numeric score and normal confidence will be shown as the worst risk instead of incomplete.
🐛 Proposed fix
const isPartial =
!report ||
!report?.scorecard ||
+ !Number.isFinite(report?.scorecard?.score) ||
report?.scorecard?.confidence === "LOW" ||
!report?.meta?.name ||
report?.meta?.name === "Unknown Extension" ||
report?.meta?.publisher === "Unknown";
@@
{!isPartial ? (
<>
- {Number.isFinite(scorecard?.score) ? scorecard.score : 0}
+ {scorecard.score}
<span style={{ fontSize: "1rem", opacity: 0.7 }}>/100</span>
</>
) : (Also applies to: 119-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/reports/ReportDetailPage.jsx` around lines 39 - 45, The
isPartial calculation currently misses validating scorecard.score so reports
with missing/non-numeric scores render as 0/100; update the isPartial logic (the
isPartial constant) to treat a missing or non-numeric report?.scorecard?.score
as partial (e.g., check typeof score === "number" and isFinite(score) or
Number.isFinite), and change any rendering that currently falls back to 0 to
show an explicit "N/A" or incomplete state when isPartial is true (also apply
the same validation where score is used later in the score rendering block
referenced near the second occurrence to avoid falling back to 0).
| report?.meta?.name === "Unknown Extension" || | ||
| report?.meta?.publisher === "Unknown"; | ||
|
|
||
| const [mode, setMode] = useState("simple"); |
There was a problem hiding this comment.
Remove or rewire the Simple/Advanced toggle.
mode is updated here but never read anywhere in ReportViewModelDetail, so both buttons are no-ops right now.
🐛 Proposed fix
- const [mode, setMode] = useState("simple");
@@
- <div style={{ marginTop: "1rem", display: "flex", gap: "0.5rem" }}>
- <Button onClick={() => setMode("simple")}>Simple</Button>
- <Button onClick={() => setMode("advanced")}>Advanced</Button>
- </div>Also applies to: 147-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/reports/ReportDetailPage.jsx` at line 47, The local state
variable mode (const [mode, setMode]) is updated by the Simple/Advanced buttons
but never read in ReportViewModelDetail, so the toggle is a no-op; fix by either
removing the unused state and the toggle buttons or wiring the state through to
where it matters: pass mode as a prop into ReportViewModelDetail (or its child
components) and use it to conditionally render the simple vs advanced UI, or
remove the setMode calls and delete the toggle markup/buttons entirely; update
any handlers tied to setMode accordingly so there are no unused state warnings.
| const handleScanClick = useCallback(() => { | ||
| if (!url.trim()) { | ||
| setError("Please enter a Chrome Web Store URL"); | ||
| return; | ||
| } | ||
| startScan(url); | ||
| const input = url.trim(); | ||
|
|
||
| if (!input) { | ||
| setError(" Please enter a Chrome Web Store URL"); | ||
| return; | ||
| } | ||
|
|
||
| const extensionId = realScanService.extractExtensionId(input); | ||
|
|
||
| if (!extensionId) { | ||
| setError("❌ Invalid Chrome Web Store URL"); | ||
| return; | ||
| } | ||
|
|
||
| setError(null); | ||
|
|
||
| // it send id instead of full url | ||
| startScan(extensionId); | ||
| }, [url, startScan, setError]); |
There was a problem hiding this comment.
Mirror the button guards inside handleScanClick().
Pressing Enter in the input still calls this callback even when the scan button is disabled, because this path never re-checks isScanning or the daily-limit gate. That allows duplicate scans and lets the keyboard path bypass the guest-limit block.
🐛 Proposed fix
const handleScanClick = useCallback(() => {
- const input = url.trim();
+ const input = url.trim();
+ const limitReached = deepScanLimit && deepScanLimit.remaining <= 0 && !cachedAvailable;
+
+ if (isScanning || limitReached) {
+ return;
+ }
if (!input) {
setError(" Please enter a Chrome Web Store URL");
return;
}
@@
- startScan(extensionId);
- }, [url, startScan, setError]);
+ startScan(extensionId);
+ }, [url, startScan, setError, isScanning, deepScanLimit, cachedAvailable]);📝 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.
| const handleScanClick = useCallback(() => { | |
| if (!url.trim()) { | |
| setError("Please enter a Chrome Web Store URL"); | |
| return; | |
| } | |
| startScan(url); | |
| const input = url.trim(); | |
| if (!input) { | |
| setError(" Please enter a Chrome Web Store URL"); | |
| return; | |
| } | |
| const extensionId = realScanService.extractExtensionId(input); | |
| if (!extensionId) { | |
| setError("❌ Invalid Chrome Web Store URL"); | |
| return; | |
| } | |
| setError(null); | |
| // it send id instead of full url | |
| startScan(extensionId); | |
| }, [url, startScan, setError]); | |
| const handleScanClick = useCallback(() => { | |
| const input = url.trim(); | |
| const limitReached = deepScanLimit && deepScanLimit.remaining <= 0 && !cachedAvailable; | |
| if (isScanning || limitReached) { | |
| return; | |
| } | |
| if (!input) { | |
| setError(" Please enter a Chrome Web Store URL"); | |
| return; | |
| } | |
| const extensionId = realScanService.extractExtensionId(input); | |
| if (!extensionId) { | |
| setError("❌ Invalid Chrome Web Store URL"); | |
| return; | |
| } | |
| setError(null); | |
| // it send id instead of full url | |
| startScan(extensionId); | |
| }, [url, startScan, setError, isScanning, deepScanLimit, cachedAvailable]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/scanner/ScannerPage.jsx` around lines 405 - 424, The
handleScanClick callback currently only validates url and extensionId and
therefore can be invoked via Enter even when the UI button is disabled; update
handleScanClick to mirror the button guards by early-returning if isScanning is
true or the daily-limit gate is reached (e.g., hasReachedDailyLimit or
canScanToday is false) before doing any validation or calling startScan; keep
existing url/extensionId validation and setError behavior, and ensure you still
call startScan(extensionId) only after these guard checks pass (use the same
boolean flags/props used to disable the button so keyboard/Enter respects the
same limits).
| const extensionId = realScanService.extractExtensionId(input); | ||
|
|
||
| if (!extensionId) { | ||
| setError("❌ Invalid Chrome Web Store URL"); | ||
| return; | ||
| } | ||
|
|
||
| setError(null); | ||
|
|
||
| // it send id instead of full url | ||
| startScan(extensionId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'ScanContext.*' frontend/src/context -x sed -n '1,260p' {}
echo '---'
rg -n -C3 'extractExtensionId|startScan\s*\(' frontend/src/context frontend/src/services frontend/src/pages/scannerRepository: Stanzin7/ExtensionShield
Length of output: 18968
Fix startScan() call to pass the full URL instead of just the extension ID.
Line 423 passes extensionId to startScan(), but startScan() in ScanContext expects a full Chrome Web Store URL. The regex in extractExtensionId() (which startScan() calls on line 174) looks for the /detail/ path segment followed by the 32-character ID—it will return null when passed a bare extension ID, causing the scan to fail with "Invalid Chrome Web Store URL format".
Pass input (the original user-provided URL) to startScan() instead:
Suggested fix
- startScan(extensionId);
+ startScan(input);📝 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.
| const extensionId = realScanService.extractExtensionId(input); | |
| if (!extensionId) { | |
| setError("❌ Invalid Chrome Web Store URL"); | |
| return; | |
| } | |
| setError(null); | |
| // it send id instead of full url | |
| startScan(extensionId); | |
| const extensionId = realScanService.extractExtensionId(input); | |
| if (!extensionId) { | |
| setError("❌ Invalid Chrome Web Store URL"); | |
| return; | |
| } | |
| setError(null); | |
| // it send id instead of full url | |
| startScan(input); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/scanner/ScannerPage.jsx` around lines 413 - 423, The code
currently calls startScan(extensionId) but startScan in ScanContext expects a
full Chrome Web Store URL and will internally call extractExtensionId(), so
replace the argument with the original user input: call startScan(input) after
validating extensionId; keep the existing setError(null) and return flow using
extensionId only for validation, i.e., use extensionId from
realScanService.extractExtensionId(input) to validate and then invoke
startScan(input) so extractExtensionId() inside startScan receives the full URL.
| const hasAnyScore = | ||
| scores?.security?.score || | ||
| scores?.privacy?.score || | ||
| scores?.governance?.score; |
There was a problem hiding this comment.
Treat 0 as a real score, not “no score”.
hasAnyScore is using truthiness, so a valid score of 0 is treated as absent. In a partial report, that hides the donut and shows “No reliable data available” for the worst possible scored result.
🐛 Proposed fix
- const hasAnyScore =
- scores?.security?.score ||
- scores?.privacy?.score ||
- scores?.governance?.score;
+ const hasAnyScore = [
+ scores?.security?.score,
+ scores?.privacy?.score,
+ scores?.governance?.score,
+ ].some((score) => Number.isFinite(score));Also applies to: 771-782
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/scanner/ScanResultsPageV2.jsx` around lines 379 - 382, The
hasAnyScore boolean currently treats 0 as falsy; change its checks to explicitly
detect presence instead of truthiness by testing for null/undefined or numeric
type. Replace the expression assigning hasAnyScore (which references
scores?.security?.score, scores?.privacy?.score, scores?.governance?.score) with
something like explicit existence checks (e.g., scores?.security?.score != null
|| scores?.privacy?.score != null || scores?.governance?.score != null) or
Number.isFinite(...) / typeof === 'number' checks so that 0 counts as a real
score; make the same change for the other analogous block that uses these same
score properties.
| const isPartialReport = | ||
| scanResults?.status === "failed" || | ||
| (scanResults?.error && | ||
| typeof scanResults.error === "string" && | ||
| scanResults.error.toLowerCase().includes("download")); |
There was a problem hiding this comment.
Use the same partial-report predicate on both results pages.
ReportDetailPage now treats low-confidence / metadata-incomplete payloads as incomplete, but this page only checks failed / "download". The same scan can therefore look incomplete on /reports/... and fully interactive here.
♻️ Suggested direction
- const isPartialReport =
- scanResults?.status === "failed" ||
- (scanResults?.error &&
- typeof scanResults.error === "string" &&
- scanResults.error.toLowerCase().includes("download"));
+ const isPartialReport = isPartialScanReport(scanResults);// e.g. shared helper
export function isPartialScanReport(scanResult) {
const scorecard = scanResult?.report_view_model?.scorecard;
const meta = scanResult?.report_view_model?.meta;
return (
scanResult?.status === "failed" ||
(typeof scanResult?.error === "string" &&
scanResult.error.toLowerCase().includes("download")) ||
scorecard?.confidence === "LOW" ||
!meta?.name ||
meta?.name === "Unknown Extension" ||
meta?.publisher === "Unknown"
);
}Also applies to: 771-782, 811-836
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/scanner/ScanResultsPageV2.jsx` around lines 486 - 490, The
partial-report detection in ScanResultsPageV2.jsx (variable isPartialReport) is
too narrow compared to ReportDetailPage and must be replaced with the shared
predicate used there; extract or import a helper (e.g., isPartialScanReport)
that checks status === "failed", error includes "download", scorecard.confidence
=== "LOW", and missing/placeholder meta fields (meta.name, "Unknown Extension",
meta.publisher "Unknown"), then use that helper wherever isPartialReport is
computed (the existing isPartialReport declaration and the other occurrences
noted around the later blocks) so both pages use the identical logic.
| """ | ||
| if db_path is None: | ||
| db_path = get_settings().database_path | ||
| db_path = Path.cwd() / "data" / "db.sqlite3" |
There was a problem hiding this comment.
Use configured database_path as default instead of Path.cwd().
Line 79 bypasses get_settings().database_path, so environment/config overrides no longer work. That can silently point SQLite to a different file and fragment persisted data across runs/deployments.
Suggested fix
if db_path is None:
- db_path = Path.cwd() / "data" / "db.sqlite3"
+ configured_db_path = get_settings().database_path
+ db_path = configured_db_path or (Path.cwd() / "data" / "db.sqlite3")📝 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.
| db_path = Path.cwd() / "data" / "db.sqlite3" | |
| if db_path is None: | |
| configured_db_path = get_settings().database_path | |
| db_path = configured_db_path or (Path.cwd() / "data" / "db.sqlite3") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/extension_shield/api/database.py` at line 79, Replace the hard-coded
Path.cwd() usage so the code uses the configured setting: read
get_settings().database_path and use it as the default base for db_path (falling
back to Path.cwd()/data/db.sqlite3 only if database_path is empty), updating the
assignment where db_path is set in src/extension_shield/api/database.py (the
variable named db_path) so environment/config overrides are respected.
| from pathlib import Path | ||
| RESULTS_DIR = Path.cwd() / "data" / "extensions_storage" # Convert to absolute path | ||
| RESULTS_DIR.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
Revert hardcoded RESULTS_DIR; it breaks configured storage contracts.
Line 1020 hardcodes storage to Path.cwd()/data/extensions_storage, bypassing get_settings() path config. This can split scan artifacts between configured storage and CWD storage, causing lookup misses and environment-dependent behavior.
Suggested fix
_settings = get_settings()
STORAGE_PATH = _settings.extension_storage_path
-from pathlib import Path
-RESULTS_DIR = Path.cwd() / "data" / "extensions_storage" # Convert to absolute path
+RESULTS_DIR = _settings.paths.results_dir
RESULTS_DIR.mkdir(parents=True, exist_ok=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/extension_shield/api/main.py` around lines 1019 - 1021, Replace the
hardcoded RESULTS_DIR that uses Path.cwd() with the configured storage root from
get_settings(): call get_settings(), read the configured storage path (e.g.,
settings.storage_path or settings.data_dir) and build RESULTS_DIR =
Path(settings.<storage_attr>) / "extensions_storage", then ensure
RESULTS_DIR.mkdir(parents=True, exist_ok=True); update the existing RESULTS_DIR
definition and mkdir call accordingly so scan artifacts use the configured
storage instead of CWD.
|
Thanks @SrashtiChauhan for working on this but this seems invalid, as there are multiple dependencies change and completely hamper the codebase |
Yes sir, i am just working on it. |
|
There shouldn't be so many commits in this one PR. Also, this seems to go against codebase standards, adds too many dependencies, and causes a lot of problems. |
I have resolved the npm audit vulnerabilities and made the necessary overrides .
Summary by CodeRabbit
New Features
Changes
Chores