fix: --fresh fails to clean cookie_storage, auth_svc crashes on key mismatch#3355
fix: --fresh fails to clean cookie_storage, auth_svc crashes on key mismatch#3355
Conversation
…ismatch - Add data/cookie_storage to DATA_FILE_GLOBS so --fresh removes it - Catch SystemExit in auth_svc when file_svc._read() fails to decrypt stale cookie_storage; regenerate session key instead of crashing - Add tests: DATA_FILE_GLOBS membership + stale cookie recovery Fixes crash when switching between --insecure and secure mode after PR #3264 introduced persistent session cookies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes --fresh cleanup and startup robustness for persistent sessions by ensuring data/cookie_storage is treated as managed state and by preventing auth_svc from crashing when the encryption key used to decrypt cookie_storage changes.
Changes:
- Add
data/cookie_storagetoDATA_FILE_GLOBSsoDataService.destroy()(used by--fresh) removes it. - Update
AuthService.apply()to recover fromcookie_storagedecryption failure by deleting the stale file and regenerating a session key instead of terminating the server. - Add tests validating glob membership and key-mismatch recovery behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/service/data_svc.py | Includes data/cookie_storage in the --fresh managed deletion globs. |
| app/service/auth_svc.py | Catches SystemExit from file_svc._read() on key mismatch and regenerates cookie_storage. |
| tests/services/test_fresh_cleanup.py | Adds regression tests for DATA_FILE_GLOBS membership and stale-cookie recovery. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Validated this fix resolves the problem of cookie storage not cleaning up with |
|
Verified clean exit when not using --fresh and switching between regular/insecure deployment, in addition to update of cookie storage when --fresh is used |
uruwhy
left a comment
uruwhy
left a comment
There was a problem hiding this comment.
see requested changes in above comments
|
❌ The last analysis has failed. |
…up since only necessary change to fix issue is adding data/cookie_storage to DATA_FILE_GLOBS
|
|
❌ The last analysis has failed. |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
|
|
reverted changes except adding the cookie storage file to the list of files deleted on --fresh. There was no need to handle the sys.exit exception to begin with, since --fresh will delete files before app_svc.apply even kicks off. So if --fresh is toggled, auth_svc.apply will always create a new cookie file and will not run into the InvalidToken issue from decryption. Verified that switching between different config files (with diff encryption keys) worked fine with --fresh and that --fresh deletes the existing cookie storage file |
|
|
sonarcloud complaint is about a section of code that's not actually part of this PR, so bypassing for now |
Summary
data/cookie_storagetoDATA_FILE_GLOBSso--freshproperly removes itauth_svcgracefully: catchSystemExitfromfile_svc._read(), regenerate session key instead of crashingProblem
After PR #3264 (persistent sessions),
data/cookie_storageis saved as an encrypted file. When the encryption key changes (e.g., switching between--insecureand secure mode),--freshfails to clean it because it's not in the managed file list. On next startup,file_svc._read()hitsInvalidTokenand callssys.exit(1), crashing the server even with--fresh.Reproduction
Changes
app/service/data_svc.pydata/cookie_storagetoDATA_FILE_GLOBSapp/service/auth_svc.pySystemExitfrom_read(), delete stale cookie, regenerate keytests/services/test_fresh_cleanup.pyTest plan
--freshnow removesdata/cookie_storageBaseWorld._app_configuration— no state leakage🤖 Generated with Claude Code