Repository navigation
feat: add remote session status API - #125
Conversation
📝 WalkthroughWalkthroughThis PR refactors HTTP API authentication to center on an ChangesAuth Context & Session Ownership
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/ui/auth.py`:
- Around line 106-112: The current code silently disables auth when
raw_server_config is not a dict; instead, if CODING_AGENT_SERVER_CONFIG is set
and raw_server_config is not a mapping, fail closed by raising an error. Change
the block that checks raw_server_config (the variable raw_server_config and the
early return that constructs _ConfiguredAuth) to detect whether
CODING_AGENT_SERVER_CONFIG is present
(os.environ.get("CODING_AGENT_SERVER_CONFIG") or equivalent) and, when present
and raw_server_config is not a dict, raise a ValueError or RuntimeError with a
clear message like "Invalid CODING_AGENT_SERVER_CONFIG: 'server' must be a
mapping" rather than returning a permissive _ConfiguredAuth.
In `@coding-agent/src/coding_agent/ui/http_server.py`:
- Around line 511-527: The access check in _auth_context_can_access_session
currently denies any non-admin auth_context access when
_session_owner_label(session) is None; update the logic to treat missing
owner_label as a legacy session and allow access for authenticated requests by
first assigning owner_label = _session_owner_label(session) and returning True
when owner_label is None (so legacy sessions remain accessible), otherwise
return owner_label == auth_context.owner_label; keep the existing early returns
for auth_context is None and admin scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3df1cc66-e18e-413d-a814-4462c8ab7e0b
📒 Files selected for processing (6)
coding-agent/src/coding_agent/ui/auth.pycoding-agent/src/coding_agent/ui/http_server.pycoding-agent/src/coding_agent/ui/schemas.pycoding-agent/src/coding_agent/ui/session_manager.pycoding-agent/tests/ui/test_http_server.pycoding-agent/tests/ui/test_security.py
| if not isinstance(raw_server_config, dict): | ||
| return _ConfiguredAuth( | ||
| bearer_token=None, | ||
| admin_bearer_token=None, | ||
| token_label_map={}, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Fail closed when explicit server config has an invalid server section type.
With CODING_AGENT_SERVER_CONFIG set, a non-mapping server value currently disables auth silently. That creates a misconfiguration fail-open path.
Suggested fix
- if not isinstance(raw_server_config, dict):
- return _ConfiguredAuth(
- bearer_token=None,
- admin_bearer_token=None,
- token_label_map={},
- )
+ if not isinstance(raw_server_config, dict):
+ raise RuntimeError("server config section must be a mapping")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@coding-agent/src/coding_agent/ui/auth.py` around lines 106 - 112, The current
code silently disables auth when raw_server_config is not a dict; instead, if
CODING_AGENT_SERVER_CONFIG is set and raw_server_config is not a mapping, fail
closed by raising an error. Change the block that checks raw_server_config (the
variable raw_server_config and the early return that constructs _ConfiguredAuth)
to detect whether CODING_AGENT_SERVER_CONFIG is present
(os.environ.get("CODING_AGENT_SERVER_CONFIG") or equivalent) and, when present
and raw_server_config is not a dict, raise a ValueError or RuntimeError with a
clear message like "Invalid CODING_AGENT_SERVER_CONFIG: 'server' must be a
mapping" rather than returning a permissive _ConfiguredAuth.
| def _session_owner_label(session: Session) -> str | None: | ||
| origin = session.origin | ||
| if origin is None: | ||
| return None | ||
| owner_label = origin.get("owner_label") | ||
| return owner_label if isinstance(owner_label, str) and owner_label else None | ||
|
|
||
|
|
||
| def _auth_context_can_access_session( | ||
| auth_context: AuthContext | None, | ||
| session: Session, | ||
| ) -> bool: | ||
| if auth_context is None: | ||
| return True | ||
| if auth_context.scope == "admin": | ||
| return True | ||
| return _session_owner_label(session) == auth_context.owner_label |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether any load/backfill path populates missing origin.owner_label
# and whether tests cover legacy unlabeled sessions.
rg -n -C3 --type=py 'owner_label|auth_scope|token_digest|backfill_owner|session\.origin|legacy.*session|unlabeled' src testsRepository: delete-cloud/Coding-Agent-Loop
Length of output: 164
🏁 Script executed:
git ls-files | head -20Repository: delete-cloud/Coding-Agent-Loop
Length of output: 1189
🏁 Script executed:
#!/bin/bash
# Find the correct directory structure for the project
find . -type d -name "coding-agent" -o -name "src" -o -name "tests" | head -20Repository: delete-cloud/Coding-Agent-Loop
Length of output: 129
🏁 Script executed:
#!/bin/bash
# List files in the coding-agent directory if it exists
if [ -d "coding-agent" ]; then
ls -la coding-agent/ | head -20
find coding-agent -type f -name "*.py" | grep -E "(test|session)" | head -20
fiRepository: delete-cloud/Coding-Agent-Loop
Length of output: 2027
🏁 Script executed:
#!/bin/bash
# Search for owner_label and related patterns in correct directories
rg -n -C3 'owner_label' coding-agent/src coding-agent/tests --type=pyRepository: delete-cloud/Coding-Agent-Loop
Length of output: 5987
🏁 Script executed:
#!/bin/bash
# Look at the session models and structures
rg -n -C3 'class.*Session|origin.*=' coding-agent/src --type=py | head -100Repository: delete-cloud/Coding-Agent-Loop
Length of output: 7419
🏁 Script executed:
#!/bin/bash
# Check for backfill, migration, or fallback logic
rg -n -C3 'backfill|migration|fallback|legacy.*session|unlabeled' coding-agent/src --type=pyRepository: delete-cloud/Coding-Agent-Loop
Length of output: 10948
🏁 Script executed:
#!/bin/bash
# View the backfill_owner_leases method implementation
sed -n '828,880p' coding-agent/src/coding_agent/ui/session_manager.pyRepository: delete-cloud/Coding-Agent-Loop
Length of output: 2248
🏁 Script executed:
#!/bin/bash
# Check what _auth_context_can_access_session does in its entirety
sed -n '519,530p' coding-agent/src/coding_agent/ui/http_server.pyRepository: delete-cloud/Coding-Agent-Loop
Length of output: 398
🏁 Script executed:
#!/bin/bash
# Search for test cases that cover legacy sessions or sessions without owner_label
rg -n -C5 'legacy.*session|session.*legacy|owner_label.*None|unlabeled.*session|without.*owner' coding-agent/tests --type=pyRepository: delete-cloud/Coding-Agent-Loop
Length of output: 23020
🏁 Script executed:
#!/bin/bash
# Search for where _auth_context_can_access_session is called and if there's any fallback
rg -n -B5 -A5 '_auth_context_can_access_session' coding-agent/src --type=pyRepository: delete-cloud/Coding-Agent-Loop
Length of output: 2928
🏁 Script executed:
#!/bin/bash
# Check if there's any logic that handles "no owner_label" case differently
rg -n -C5 'GET /sessions|get_sessions' coding-agent/src/coding_agent/ui/http_server.py --type=py | head -80Repository: delete-cloud/Coding-Agent-Loop
Length of output: 56
Authenticated non-admin users will be locked out of sessions created before this PR.
The strict equality check in _auth_context_can_access_session() (line 527) denies access to any session where _session_owner_label() returns None. Legacy sessions without origin["owner_label"] will fail this check, making them inaccessible via GET /sessions and GET /sessions/{session_id} even for their original owners.
The backfill_owner_leases() method (called on startup at line 430) only populates the owner lease table, not the origin["owner_label"] field in persisted sessions. Without a migration to backfill missing labels or a fallback in the access logic, this creates a breaking change on rollout. Either populate origin["owner_label"] for existing sessions during startup, or add a fallback condition (e.g., allow access if auth_context is set but origin.owner_label is missing).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@coding-agent/src/coding_agent/ui/http_server.py` around lines 511 - 527, The
access check in _auth_context_can_access_session currently denies any non-admin
auth_context access when _session_owner_label(session) is None; update the logic
to treat missing owner_label as a legacy session and allow access for
authenticated requests by first assigning owner_label =
_session_owner_label(session) and returning True when owner_label is None (so
legacy sessions remain accessible), otherwise return owner_label ==
auth_context.owner_label; keep the existing early returns for auth_context is
None and admin scope.
Summary
Validation
Summary by CodeRabbit
New Features
Refactor
Tests