extension icons should appear #11143
Conversation
| @@ -24,6 +24,13 @@ module.exports.fileUrl = (filePath) => { | |||
| return encodeURI('file://' + fileUrlPath) | |||
| } | |||
|
|
|||
| module.exports.chromeUrl = (filePath = '') => { | |||
There was a problem hiding this comment.
would be good to have some unit tests for this
There was a problem hiding this comment.
added in 1bc13ba
There was a problem hiding this comment.
can we use a relative path instead and make it filesystem agnostic?
| @@ -24,6 +24,13 @@ module.exports.fileUrl = (filePath) => { | |||
| return encodeURI('file://' + fileUrlPath) | |||
| } | |||
|
|
|||
| module.exports.chromeUrl = (filePath = '') => { | |||
| filePath = module.exports.fileUrl(filePath) | |||
| filePath = filePath.replace('file://', 'chrome://brave') | |||
There was a problem hiding this comment.
where is the chrome://brave to file:// mapping implemented in muon? it seems like a potential security hole since it can be used to bypass CSP restrictions; we should make sure chrome://brave loads are blocked in untrusted contexts.
There was a problem hiding this comment.
also i'm curious what CSP violations are being triggered? app/extensions/brave/index.html explicitly allows img-src file: but not chrome: so i'm confused why the original issue happens
There was a problem hiding this comment.
I played with the CSP file but couldn't get the violation to reveal itself either. It might be some other security restriction. The error from the console is
[75566:775:0922/145254.257520:ERROR:CONSOLE(0)] "Not allowed to load local resource:
file:///Users/user/Desktop/brave/browser-laptop-bootstrap/src/browser-laptop/app/extensions/metalmash/images/icon-19.png",
source: chrome://brave/Users/user/Desktop/brave/browser-laptop-bootstrap/src/browser-laptop/app/extensions/brave/index-dev.html (0)
and it disappears when switching to chrome://brave. Subsequently the image loads.
There was a problem hiding this comment.
@diracdeltas chrome://brave can't be loaded from web pages Not allowed to load local resource: chrome://brave/usr/local/brave/browser-laptop-bootstrap/README.md. When I originally tested them it also wasn't possible to load them by entering the url manually, but I just tested again and it is now possible because we added support for chrome://* urls in general so we should probably block them from the urlbar
There was a problem hiding this comment.
Also, the csp can't allow something that is prohibited by default (access to file resources) which is why the file:// urls don't work
There was a problem hiding this comment.
I think it's fine if chrome://brave URLs can be loaded manually from the urlbar since that is also possible for file URLs. just want to make sure there is nothing they can do that files URLs can't do, other than be loaded in the chrome:// context.
kevinlawler
force-pushed
the
fix/chrome-url-extensions-icons
branch
from
b0b367b to
1bc13ba
Compare
|
what's the current status of this? |
cezaraugusto
added
the
needs-info
bbondy
modified the milestones:
0.21.x (Developer Channel),
0.20.x (Beta Channel),
Backlog
changes file:// to chrome://brave Issue #11142 Auditors: @bridiver @diracdeltas @darkdh @jonathansampson fix whitespace add unit test
bsclifton
force-pushed
the
fix/chrome-url-extensions-icons
branch
from
1bc13ba to
a51b7ea
Compare
bsclifton
changed the title
|
Submitted security review after seeing the above text: |
extension icons should appear
extension icons should appear
changes
file://tochrome://bravefor extension icon loadingIssue #11142
Auditors:
@bridiver @diracdeltas @darkdh @jonathansampson