Repository navigation
Conversation
|
Merging this PR will not alter performance
|
514269c to
b7efd10
Compare
b227f18 to
427f2c0
Compare
427f2c0 to
db30d29
Compare
Introduce a stdlib HS256 session codec, environment-only signing keys with rotation, and persistent local secrets excluded from exports. Preserve the local secret during web reinitialization. Validation: 421 affected tests passed, plus unknown-key diagnostic coverage; Ruff and targeted pyright passed. Refs: ENG-12910
db30d29 to
44c4b4e
Compare
There was a problem hiding this comment.
2 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="reflex/utils/frontend_skeleton.py">
<violation number="1" location="reflex/utils/frontend_skeleton.py:471">
P1: This branch follows a symlinked `.web/backend` and removes its target's contents, which can erase files outside the generated web directory during `reflex init`. Guard against symlink traversal and handle the link without modifying its target.</violation>
</file>
<file name="tests/units/reflex_base/test_session.py">
<violation number="1" location="tests/units/reflex_base/test_session.py:421">
P2: This defaults assertion fails whenever either session setting is already present in the test process environment. Clear both variables before constructing the default config so the test remains isolated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| path_ops.copy_tree(constants.Templates.Dirs.WEB_TEMPLATE, str(web_dir)) | ||
| if web_dir.exists(): | ||
| for item in web_dir.iterdir(): | ||
| if item.name == constants.Dirs.BACKEND and item.is_dir(): |
There was a problem hiding this comment.
P1: This branch follows a symlinked .web/backend and removes its target's contents, which can erase files outside the generated web directory during reflex init. Guard against symlink traversal and handle the link without modifying its target.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At reflex/utils/frontend_skeleton.py, line 471:
<comment>This branch follows a symlinked `.web/backend` and removes its target's contents, which can erase files outside the generated web directory during `reflex init`. Guard against symlink traversal and handle the link without modifying its target.</comment>
<file context>
@@ -464,7 +466,17 @@ def initialize_web_directory():
- path_ops.copy_tree(constants.Templates.Dirs.WEB_TEMPLATE, str(web_dir))
+ if web_dir.exists():
+ for item in web_dir.iterdir():
+ if item.name == constants.Dirs.BACKEND and item.is_dir():
+ for backend_file in item.iterdir():
+ if backend_file.name != SESSION_SECRET_FILENAME:
</file context>
| monkeypatch: The environment override fixture. | ||
| """ | ||
| monkeypatch.setenv("REFLEX_SESSION_SECRET", SECRET.decode()) | ||
| config = Config(app_name="test") |
There was a problem hiding this comment.
P2: This defaults assertion fails whenever either session setting is already present in the test process environment. Clear both variables before constructing the default config so the test remains isolated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/units/reflex_base/test_session.py, line 421:
<comment>This defaults assertion fails whenever either session setting is already present in the test process environment. Clear both variables before constructing the default config so the test remains isolated.</comment>
<file context>
@@ -0,0 +1,446 @@
+ monkeypatch: The environment override fixture.
+ """
+ monkeypatch.setenv("REFLEX_SESSION_SECRET", SECRET.decode())
+ config = Config(app_name="test")
+ assert config.session_token_ttl == 604800
+ assert config.session_token_refresh_interval is None
</file context>
| config = Config(app_name="test") | |
| monkeypatch.delenv("REFLEX_SESSION_TOKEN_TTL", raising=False) | |
| monkeypatch.delenv("REFLEX_SESSION_TOKEN_REFRESH_INTERVAL", raising=False) | |
| config = Config(app_name="test") |
Add signed browser sessions and cryptographically bound per-tab client tokens. A client token alone cannot prove ownership of another session. Credentials are bound to the application's name, use HS256 with a pinned algorithm, and validate schema, lifetime, signing-key ID, and audience. Rotation preserves existing tab bindings.
REFLEX_SESSION_SECRETsupplies a comma-separated signing-key ring. Without it, workers share an atomically published key in.web/backend/session_secret; initialization preserves it and exported archives exclude it. POSIX files use owner-only permissions; Windows follows inherited directory ACLs. Config exposes a seven-day default TTL and half-TTL refresh interval; enforcement modes areoff,warn(default), andenforce.Rebased on main
d02ba5409. Review order: #7361 → #7362 → #7363 → #7364. The earlier #7360 dependency is merged.Validation: 244 focused session, initialization, export, and locking tests passed. Audience-isolation and persistence regressions fail without their fixes. The full assembled stack passes 12,147 unit tests with 80.15% coverage; see #7364 for integration validation.
Session ownership is intentionally valid for an already established connection; expiry gates HTTP credentials and reconnects. This is not application login or per-session revocation. The review discussion records the rationale.
Related to ENG-12910, ENG-9088.