-
Notifications
You must be signed in to change notification settings - Fork 36
[kernel 726] fix web bot auth extension #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[kernel 726] fix web bot auth extension #110
Conversation
de5debc to
1af7c34
Compare
1af7c34 to
f9e1cee
Compare
rgarcia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good progress on the Chrome enterprise policy implementation. The core logic for separating policy vs non-policy extensions looks solid.
Main concern: the deleted TestWebBotAuthInstallation test should be updated rather than removed - the new flow has significant behavior changes that warrant e2e coverage.
A few minor nits on style/perf, and a question about whether the /update.xml root route is necessary.
| return targets, nil | ||
| } | ||
|
|
||
| func TestWebBotAuthInstallation(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test was verifying the web-bot-auth policy installation flow. rather than deleting it, could we update it to test the new behavior? the new flow (requiring update.xml + .crx, ExtensionInstallForcelist, update_url instead of path) seems important enough to have e2e coverage.
rgarcia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes since last review look good:
- All nits addressed (idiomatic error declaration,
strings.HasPrefix, package-level regex) - Removed non-deterministic
/update.xmland/{filename}.crxroutes - Re-added
TestWebBotAuthInstallationwith proper update.xml and .crx files RuntimeAllowedHostsremoval is correct - matches upstream web-bot-auth reference and force-installed extensions get manifest permissions automatically
Ship it 🚀
Note
Introduces enterprise policy-based installation for extensions and serves required files, improving reliability for extensions needing
webRequest/webRequestBlocking.UploadExtensionsAndRestart, detect policy-required extensions, extract Chrome extension ID fromupdate.xml, validate presence ofupdate.xmland.crx, update enterprise policy, and only add--load-extensionflags for non-policy extensionsExtensionInstallForcelist, useupdate_urlinstead of localpath, dedupe forcelist entries, and addExtractExtensionIDFromUpdateXMLwith ID validationhttp://.../extensions/*from/home/kernel/extensionsso Chrome can fetchupdate.xmland.crxTestWebBotAuthInstallationverifyingExtensionInstallForcelistand extension directory/chromium/flagsbind mount writableWritten by Cursor Bugbot for commit 46a6e1e. This will update automatically on new commits. Configure here.