Add Permission Risk Explanation Engine & UI Breakdowns#155
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a frontend “permission risk explanation” breakdown so that flagged permission-related factors show the specific permissions/combos driving the risk along with human-readable explanations.
Changes:
- Added a
PERMISSION_RISKSmapping to translate Chrome permissions / host patterns into plain-English impact descriptions. - Updated
LayerModalcheck rows to conditionally render expanded breakdown sections forPermissionsBaselineandPermissionCombos. - Adjusted check-row layout to accommodate multi-line details under a factor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="lm-check-left" style={{ flexDirection: 'column', alignItems: 'flex-start', gap: '4px' }}> | ||
| <div style={{ display: 'flex', alignItems: 'center', gap: '6px' }}> | ||
| <span className="lm-check-name">{item.label}</span> | ||
| {item.desc && <InfoTooltip text={item.desc} />} | ||
| </div> |
There was a problem hiding this comment.
The new inline style on .lm-check-left forces a column layout for all check rows, overriding the existing .lm-check-left flex row styling defined in LayerModal.scss. This is likely to change alignment/truncation behavior for checks that don’t have permission breakdowns. Consider keeping .lm-check-left’s SCSS-driven row layout and only switching to a column layout conditionally (e.g., when a breakdown is present) via a modifier class or an inner wrapper.
| <div style={{ marginTop: '2px', paddingLeft: '8px', borderLeft: '2px solid var(--risk-warn)', fontSize: '11px', color: 'var(--theme-text-muted)', display: 'flex', flexDirection: 'column', gap: '4px' }}> | ||
| {item.raw.details.high_risk_permissions.map(perm => ( | ||
| <div key={perm} style={{ display: 'flex', gap: '6px', lineHeight: '1.2' }}> | ||
| <span style={{ fontWeight: 600, color: 'var(--risk-warn)', flexShrink: 0 }}>{perm}</span> | ||
| <span style={{ opacity: 0.6 }}>→</span> |
There was a problem hiding this comment.
The permission breakdown uses a large inline style object (and the same style is duplicated again for the combos breakdown). To keep theming/maintenance consistent with the rest of LayerModal.scss, consider moving these styles into a dedicated CSS class and reusing it. Also, the border currently uses var(--risk-warn); the SCSS elsewhere uses --risk-warn-border (and often provides a fallback), which is likely the more appropriate token for borders.
| const isBroad = combo === 'broad_host_access'; | ||
| const comboName = isBroad ? 'Broad Host Access' : combo.split('+').join(' + '); | ||
| const comboDesc = isBroad |
There was a problem hiding this comment.
comboName is derived directly from the backend’s triggered_combos string. In src/extension_shield/scoring/normalizers.py, combos are built via "+".join(required_perms) where required_perms is a set, so the order can be non-deterministic. To avoid inconsistent UI labels across runs, consider normalizing on the frontend by splitting on +, sorting, then joining for display.
| const comboDesc = isBroad | ||
| ? 'Can access and modify data on all websites' | ||
| : 'High risk when these are used together'; |
There was a problem hiding this comment.
The PR description says the “Dangerous Combos” section uses the same explanation engine to clearly state why each combination is high-risk, but the UI currently renders a generic description ('High risk when these are used together') for all non-broad-host combos. If the intent is to provide actionable context, consider mapping specific combos (or the combo’s constituent permissions via PERMISSION_RISKS) to concrete explanations rather than a one-size-fits-all message.
|
@codexuttam can you please fix the failing jobs before we can start with merging |
|
@codexuttam please fix the failed tests |
📝 WalkthroughWalkthroughThe pull request enhances the LayerModal component by introducing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
github-actions
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/components/report/LayerModal.jsx (2)
199-216: Make combo explanations specific per combo, not generic fallback text.Most combos currently render the same description (
High risk when these are used together), which weakens the “explanation engine” value. Consider a combo-risk dictionary (same pattern asPERMISSION_RISKS) with a fallback.Suggested refactor
+const PERMISSION_COMBO_RISKS = { + 'cookies+webRequest': 'Can read session cookies and intercept traffic, enabling account/session hijack patterns', + 'cookies+webRequestBlocking': 'Can modify requests while accessing cookies, increasing credential theft/manipulation risk', + broad_host_access: 'Can access and modify data on all websites', +}; ... - const comboDesc = isBroad - ? 'Can access and modify data on all websites' - : 'High risk when these are used together'; + const comboDesc = PERMISSION_COMBO_RISKS[combo] + || 'High risk when these permissions are combined';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/report/LayerModal.jsx` around lines 199 - 216, The combo display for PermissionCombos uses a generic fallback description; add a dedicated combo-to-description mapping (e.g., COMBO_RISKS) similar to PERMISSION_RISKS and use it when rendering item.raw.details.triggered_combos: for each combo in the map look up a specific description, set comboDesc = COMBO_RISKS[combo] || 'High risk when these are used together' (keep the existing special-case for 'broad_host_access' to produce 'Broad Host Access'), and replace the current hardcoded comboDesc logic in the map callback so each combo renders a precise explanation.
179-216: Consider moving repeated inline styles to SCSS classes.This block now carries many inline style objects, which increases render noise and makes future styling changes harder. Extracting these into
LayerModal.scsswould improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/report/LayerModal.jsx` around lines 179 - 216, The JSX in LayerModal.jsx (inside the lm-check-left block and the two conditional blocks for Permission Risk Explanation Engine and Dangerous Combos Explanation Engine) uses many repeated inline style objects; extract these into LayerModal.scss and replace the style props with semantic classNames. Create classes like .risk-block (for the outer div with marginTop/paddingLeft/border/fontSize/color/layout), .risk-item (for each row), .risk-label (fontWeight/color/flexShrink), .risk-arrow (opacity), and .risk-text (lineHeight/opacity), then update the map renderings (the item.raw.details.high_risk_permissions mapping and the item.raw.details.triggered_combos mapping) to use those classNames instead of inline styles; keep existing className "lm-check-left" and the PERMISSION_RISKS usage the same. Ensure SCSS uses the same variable names (e.g., --risk-warn, --theme-text-muted) so visual parity is preserved.
🤖 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/components/report/LayerModal.jsx`:
- Around line 28-50: PERMISSION_RISKS is missing an entry for the
backend-provided high-risk key browsingData so users fall back to a generic
message; add a specific mapping in the PERMISSION_RISKS object (the same object
that contains keys like webRequest and cookies) with a plain-English explanation
such as that browsingData can read or clear browsing history, cache, cookies and
other local browsing data, ensuring the UI displays this high-risk description
when high_risk_permissions includes browsingData.
---
Nitpick comments:
In `@frontend/src/components/report/LayerModal.jsx`:
- Around line 199-216: The combo display for PermissionCombos uses a generic
fallback description; add a dedicated combo-to-description mapping (e.g.,
COMBO_RISKS) similar to PERMISSION_RISKS and use it when rendering
item.raw.details.triggered_combos: for each combo in the map look up a specific
description, set comboDesc = COMBO_RISKS[combo] || 'High risk when these are
used together' (keep the existing special-case for 'broad_host_access' to
produce 'Broad Host Access'), and replace the current hardcoded comboDesc logic
in the map callback so each combo renders a precise explanation.
- Around line 179-216: The JSX in LayerModal.jsx (inside the lm-check-left block
and the two conditional blocks for Permission Risk Explanation Engine and
Dangerous Combos Explanation Engine) uses many repeated inline style objects;
extract these into LayerModal.scss and replace the style props with semantic
classNames. Create classes like .risk-block (for the outer div with
marginTop/paddingLeft/border/fontSize/color/layout), .risk-item (for each row),
.risk-label (fontWeight/color/flexShrink), .risk-arrow (opacity), and .risk-text
(lineHeight/opacity), then update the map renderings (the
item.raw.details.high_risk_permissions mapping and the
item.raw.details.triggered_combos mapping) to use those classNames instead of
inline styles; keep existing className "lm-check-left" and the PERMISSION_RISKS
usage the same. Ensure SCSS uses the same variable names (e.g., --risk-warn,
--theme-text-muted) so visual parity is preserved.
🪄 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: c81818f5-e7be-4c85-8c57-2cd5881772fd
📒 Files selected for processing (1)
frontend/src/components/report/LayerModal.jsx
| const PERMISSION_RISKS = { | ||
| tabs: 'Can read browsing activity', | ||
| webRequest: 'Can intercept and modify traffic (high risk)', | ||
| webRequestBlocking: 'Can block and modify network requests (high risk)', | ||
| cookies: 'Can read and modify site cookies', | ||
| history: 'Can read full browsing history', | ||
| clipboardRead: 'Can read copied text from clipboard', | ||
| clipboardWrite: 'Can modify clipboard contents', | ||
| desktopCapture: 'Can record your screen', | ||
| tabCapture: 'Can record browser tabs', | ||
| nativeMessaging: 'Can communicate with desktop apps', | ||
| proxy: 'Can route all traffic through external servers', | ||
| debugger: 'Can bypass security and monitor page internals', | ||
| management: 'Can disable or uninstall other extensions', | ||
| geolocation: 'Can access physical location', | ||
| bookmarks: 'Can read and modify bookmarks', | ||
| '<all_urls>': 'Can access data on all websites you visit', | ||
| '*://*/*': 'Can access data on all websites you visit', | ||
| 'http://*/*': 'Can access data on all HTTP websites', | ||
| 'https://*/*': 'Can access data on all HTTPS websites', | ||
| activeTab: 'Can access the current active tab', | ||
| storage: 'Can store data locally', | ||
| }; |
There was a problem hiding this comment.
Add missing high-risk explanation for browsingData.
high_risk_permissions can include browsingData (from the backend risk set), but this map has no specific entry for it, so users get the generic fallback instead of a plain-English risk explanation.
Proposed patch
const PERMISSION_RISKS = {
tabs: 'Can read browsing activity',
webRequest: 'Can intercept and modify traffic (high risk)',
webRequestBlocking: 'Can block and modify network requests (high risk)',
cookies: 'Can read and modify site cookies',
+ browsingData: 'Can clear browsing data (history, cache, cookies) and hide traces of activity',
history: 'Can read full browsing history',
clipboardRead: 'Can read copied text from clipboard',📝 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 PERMISSION_RISKS = { | |
| tabs: 'Can read browsing activity', | |
| webRequest: 'Can intercept and modify traffic (high risk)', | |
| webRequestBlocking: 'Can block and modify network requests (high risk)', | |
| cookies: 'Can read and modify site cookies', | |
| history: 'Can read full browsing history', | |
| clipboardRead: 'Can read copied text from clipboard', | |
| clipboardWrite: 'Can modify clipboard contents', | |
| desktopCapture: 'Can record your screen', | |
| tabCapture: 'Can record browser tabs', | |
| nativeMessaging: 'Can communicate with desktop apps', | |
| proxy: 'Can route all traffic through external servers', | |
| debugger: 'Can bypass security and monitor page internals', | |
| management: 'Can disable or uninstall other extensions', | |
| geolocation: 'Can access physical location', | |
| bookmarks: 'Can read and modify bookmarks', | |
| '<all_urls>': 'Can access data on all websites you visit', | |
| '*://*/*': 'Can access data on all websites you visit', | |
| 'http://*/*': 'Can access data on all HTTP websites', | |
| 'https://*/*': 'Can access data on all HTTPS websites', | |
| activeTab: 'Can access the current active tab', | |
| storage: 'Can store data locally', | |
| }; | |
| const PERMISSION_RISKS = { | |
| tabs: 'Can read browsing activity', | |
| webRequest: 'Can intercept and modify traffic (high risk)', | |
| webRequestBlocking: 'Can block and modify network requests (high risk)', | |
| cookies: 'Can read and modify site cookies', | |
| browsingData: 'Can clear browsing data (history, cache, cookies) and hide traces of activity', | |
| history: 'Can read full browsing history', | |
| clipboardRead: 'Can read copied text from clipboard', | |
| clipboardWrite: 'Can modify clipboard contents', | |
| desktopCapture: 'Can record your screen', | |
| tabCapture: 'Can record browser tabs', | |
| nativeMessaging: 'Can communicate with desktop apps', | |
| proxy: 'Can route all traffic through external servers', | |
| debugger: 'Can bypass security and monitor page internals', | |
| management: 'Can disable or uninstall other extensions', | |
| geolocation: 'Can access physical location', | |
| bookmarks: 'Can read and modify bookmarks', | |
| '<all_urls>': 'Can access data on all websites you visit', | |
| '*://*/*': 'Can access data on all websites you visit', | |
| 'http://*/*': 'Can access data on all HTTP websites', | |
| 'https://*/*': 'Can access data on all HTTPS websites', | |
| activeTab: 'Can access the current active tab', | |
| storage: 'Can store data locally', | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/report/LayerModal.jsx` around lines 28 - 50,
PERMISSION_RISKS is missing an entry for the backend-provided high-risk key
browsingData so users fall back to a generic message; add a specific mapping in
the PERMISSION_RISKS object (the same object that contains keys like webRequest
and cookies) with a plain-English explanation such as that browsingData can read
or clear browsing history, cache, cookies and other local browsing data,
ensuring the UI displays this high-risk description when high_risk_permissions
includes browsingData.
What I found
The current permission analysis successfully identifies and flags risky permissions (e.g., showing a generic "Issue" status), but leaves users in the dark by not clearly explaining why a specific permission or combination is dangerous. A raw flag without context isn't immediately actionable for the average user.
What I did
Implemented a Permission Risk Explanation Engine: Added a comprehensive dictionary mapping in the frontend that translates arcane Chrome API permissions (e.g., webRequestBlocking, clipboardRead) directly into human-readable impacts (e.g., "Can block and modify network requests (high risk)").
Enhanced the "Permission Risk" UI Element: Overhauled the
LayerModal
components so that when a "Permission Risk" is flagged, it dynamically expands to reveal the exact high-risk permissions causing the flag and prints their plain-English explanations side-by-side.
Added a "Dangerous Combos" Breakdown: Extended the exact same explanation engine to the Dangerous Combos layer. Instead of a generic warning, it now visually breaks out the precise toxic combinations (e.g., cookies + webRequest) and clearly states why they are high-risk when used together.
Why this matters
Users and judges need clarity over opacity. By proactively mapping technical permissions to human-readable risks, we help users understand the actual capability of the extension, bridging the gap between raw scan data and usable security advice.
Summary by CodeRabbit