fix(auth): swap session rules atomically to prevent intermittent 401s - #897
Conversation
configureAuthAndCorsHeaders() cleared all declarativeNetRequest session rules in one updateSessionRules call, then re-added them in a second call after async work. Any request in flight during that gap got no auth token injected and 401'd — both top-level navigations and JS/CSS/font sub-requests. Because the MV3 service worker re-runs this at top level on every cold start (woken by tab/navigation events), the gap recurred throughout normal browsing, not just at login.
Combine removal and addition into a single atomic updateSessionRules({ removeRuleIds, addRules }) call. DNR session rules survive worker restarts, so existing rules stay live through the rebuild and the swap is gapless. Drop the addRules.length guard so logout still clears stale rules within the same atomic call.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This is entirely AI generated, so view it with appropriate amounts of skepticism, but running with it locally for a few days and it does seem to resolve the problems I was having around random 401s when sidekick fails to append the token. |
|
@shsteimer The problem description and proposed solution makes sense to me. I tried it out locally with a few protected sites and didn't notice any negative side effects. The code diff looks reasonably isolated, too. I think we can give it a go, but should definetely keep an eye out for auth related bug reports in customer channels in the days following the deployment. |
|
🎉 This PR is included in version 7.33.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
2 participants
Problem
configureAuthAndCorsHeaders()cleared alldeclarativeNetRequestsession rules in oneupdateSessionRulescall, then re-added them in a second call after async work (reading storage + building rules). Any request dispatched during that window got no auth token injected and returned 401 — both top-level navigations and JS/CSS/font sub-requests, so pages loaded partially or failed outright.Because the MV3 background service worker is ephemeral and re-runs this setup at the top level (
background.js) on every cold start — woken by tab/navigation events — the gap recurred during normal browsing, not just at login. Classic symptom: view a page for a while (worker idles out), click a link, that load fails; reload and it's fine.Fix
Combine the removal and addition into a single atomic
updateSessionRules({ removeRuleIds, addRules })call. DNR session rules survive worker restarts, so the existing rules stay live through the rebuild and the swap is gapless — on a cold start the token is unchanged, so it's a functional no-op.Dropped the
addRules.length > 0guard: guarding the now-fused call onaddRuleswould skip removal too, leaving stale rules after logout. An unconditional single call always clears + sets in one atomic step.Scope kept intentionally minimal — the random rule IDs (added in #249 to fix a concurrent-counter collision) are left untouched; atomicity already makes a failed batch fail safe (old rules retained rather than wiped to zero).
Test
Added a reproduction test that models the browser's persistent session ruleset and asserts a site request stays authed throughout a reconfiguration. It fails on the old two-call code — observing
[ undefined, 'token …' ], i.e. the request losing then regaining the token — and passes with the atomic swap.Updated existing assertions that encoded the two-call pattern (call counts 4→2 / 5→3;
calledWithnow includesremoveRuleIds).Full suite: 759 passed, 0 failed, coverage above threshold.
🤖 Generated with Claude Code