feat(auth): add API key authentication for CLI and REST API - #326
Conversation
Implement dual authentication supporting both JWT tokens and API keys. API keys enable programmatic access with scope-based permissions. New features: - API key generation with SHA256 hashing (industry standard for high-entropy secrets) - Scope hierarchy: admin → write → read - REST endpoints: POST/GET/DELETE /api/auth/api-keys - Dual auth dependency accepting either JWT or X-API-Key header - Prefix-based lookup optimization for fast authentication Security considerations: - API key creation requires JWT auth (prevents privilege escalation) - Keys hashed with SHA256, constant-time comparison via hmac.compare_digest - Key shown only once at creation, never retrievable again - Ownership verified on revoke/delete operations Test coverage: 68 new tests following TDD principles
Add CLI commands for API key management that share business logic with REST endpoints via ApiKeyService: - cf auth api-key-create: Create new API key with scopes - cf auth api-key-list: List user's API keys (masked values) - cf auth api-key-revoke: Revoke an API key with confirmation - cf auth api-key-rotate: Rotate key (revoke old, create new) Core changes: - Extract ApiKeyService to codeframe/core/api_key_service.py - Refactor api_key_router.py to use ApiKeyService - Add get_db_for_cli() helper for CLI database access This ensures 1-1 mapping between CLI and REST API with shared business logic, eliminating duplication and potential drift. Test results: 85 tests pass (14 endpoints + 16 repo + 23 utils + 15 dual auth + 7 integration + 10 CLI commands)
WalkthroughAdds a complete API key system: key generation/verification, DB schema and repository, service layer, REST endpoints (create/list/revoke), dual JWT / X-API-Key authentication with scope enforcement, CLI commands, and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIRouter as API Key Router
participant AuthDeps as Auth Dependencies
participant Service as ApiKeyService
participant Repo as APIKeyRepository
participant Database as DB
Client->>APIRouter: POST /api/auth/api-keys (JWT + body)
APIRouter->>AuthDeps: get_current_user(JWT)
AuthDeps->>Database: validate JWT / lookup user
AuthDeps-->>APIRouter: User
APIRouter->>Service: create_api_key(user_id, name, scopes, expires_at)
Service->>Service: validate_scopes & generate_api_key
Service->>Repo: create(user_id, name, key_hash, prefix, scopes, expires_at)
Repo->>Database: INSERT api_keys
Database-->>Repo: key_id
Repo-->>Service: key_id
Service-->>APIRouter: CreatedApiKey (full key, id, prefix)
APIRouter-->>Client: 200 CreateApiKeyResponse
sequenceDiagram
participant Client
participant APIRouter as API Router
participant AuthDeps as Auth Dependencies
participant Repo as APIKeyRepository
participant Service as ApiKeyService
participant Database as DB
Client->>APIRouter: GET /api/auth/api-keys (X-API-Key header)
APIRouter->>AuthDeps: get_api_key_auth(X-API-Key)
AuthDeps->>AuthDeps: extract_prefix(key)
AuthDeps->>Repo: get_by_prefix(prefix)
Repo->>Database: SELECT api_keys WHERE prefix AND is_active
Database-->>Repo: key_row
Repo-->>AuthDeps: key_row (includes key_hash, scopes, user_id)
AuthDeps->>AuthDeps: verify_api_key(key, key_hash)
AuthDeps->>Repo: update_last_used(key_id)
AuthDeps-->>APIRouter: auth dict (type, user_id, scopes, key_id)
APIRouter->>Service: list_api_keys(user_id)
Service->>Repo: list_user_keys(user_id)
Repo->>Database: SELECT api_keys WHERE user_id
Database-->>Repo: rows
Repo-->>Service: api key list (safe)
Service-->>APIRouter: List[ApiKeyInfoResponse]
APIRouter-->>Client: 200 List response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Add API key authentication for CLI and REST API and expose
|
PR Review: API Key Authentication SystemThis is a well-architected implementation of API key authentication that properly follows the v2 guidelines. The code quality is high with strong separation of concerns and excellent test coverage (85 tests). ✅ Strengths1. Architecture Compliance
2. Security Implementation
3. Scope-Based Permissions
4. Dual Authentication Design
5. Test Coverage
🔍 Issues & Recommendations1. Database Connection Pattern (Minor - Consistency)The REST API uses request state for database connections: # codeframe/auth/api_key_router.py:87-98
def get_db(request: Request) -> Database:
db = getattr(request.state, "db", None)
if db is None:
db_path = os.getenv("DATABASE_PATH", ...)
db = Database(db_path)
db.initialize()
request.state.db = db
return dbThe CLI uses a standalone helper: # codeframe/cli/auth_commands.py:54-69
def get_db_for_cli() -> Database:
db_path = os.getenv("DATABASE_PATH", ...)
db = Database(db_path)
db.initialize()
return dbRecommendation: Document this pattern or consider extracting to a shared location if this pattern repeats. The duplication is minor but could be centralized. 2. API Key Verification Updates Last Used (Minor - Side Effect)In try:
db.api_keys.update_last_used(key_record["id"])
except Exception as e:
logger.warning(f"Failed to update last_used_at: {e}")Observation: This is a fire-and-forget write during read operations. While logged, it could silently fail. Consider:
3. Schema Migration Not Shown (Documentation)The PR adds the Check: Verify that:
4. API Key Cannot Create API Keys (Security - Documented but Consider Rotation)The PR correctly prevents API keys from creating new API keys (prevents privilege escalation). However: Question: Should API keys be able to rotate themselves? Currently
Current design is secure by default, just document the workflow. 5. Expiration Not Enforced (Implementation Gap)The schema supports # codeframe/persistence/repositories/api_key_repository.py:93-110
def get_by_prefix(self, prefix: str) -> Optional[Dict[str, Any]]:
"""Get an active API key by its prefix.
Only returns active, non-expired keys for authentication.
"""The docstring says "non-expired" but the implementation needs verification. Check that the SQL query includes: WHERE is_active = 1 AND (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)Action: Verify expiration enforcement in 6. CLI User ID Requirement (UX - Consider Enhancement)CLI commands require cf auth api-key-create --name "My Key" --user-id 1Future enhancement: Consider allowing users to omit cf auth api-key-create --name "My Key" # Uses current userNot a blocker, but would align with the 📋 Testing Recommendations
🎯 VerdictApproved with minor recommendations This is production-ready code that:
The issues identified are minor and mostly documentation/enhancement opportunities. The security fundamentals are solid. Suggested follow-up issues:
Great work on maintaining the v2 core-first, headless architecture! 🚀 |
Updated PR Review: API Key Authentication SystemFollowing up on my previous review - excellent work addressing the core implementation! ✅ Issues Resolved from Previous Review1. Expiration Enforcement ✓ 2. Schema Migration ✓ 🎯 Architecture Compliance - ExcellentThis implementation exemplifies v2 principles:
🔐 Security Implementation - Solid
📊 Test Coverage - Comprehensive85 tests covering utilities, repository, dual auth, endpoints, CLI commands, and integration. 💡 Minor Observations (No Blockers)
🚀 Verdict: LGTM - Ready to MergeThis PR delivers production-ready API key authentication with strong security, clean v2 architecture, comprehensive tests, and both CLI and REST API support. Exemplary v2 work! 🎉 Recommended merge once CI passes. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| codeframe auth api-key-list --user-id 1 | ||
| """ | ||
| # Get database and service | ||
| db = get_db_for_cli() |
There was a problem hiding this comment.
🟢 Low
codeframe/cli/auth_commands.py:853
Consider closing Database connections from get_db_for_cli() (use a context manager or call db.close()), to avoid leaked connections.
🚀 Want me to fix this? Reply ex: "fix it for me".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
codeframe/persistence/database.py (1)
216-224:⚠️ Potential issue | 🟡 MinorMissing
api_keysrepository in async connection update list.The
_update_repository_async_connectionsmethod updates async connections for all repositories but does not includeself.api_keys. This will cause the api_keys repository to have a stale async connection after reconnection.🐛 Proposed fix
def _update_repository_async_connections(self) -> None: """Update async connections in all repositories.""" for repo in [self.projects, self.issues, self.tasks, self.agents, self.blockers, self.memories, self.context_items, self.checkpoints, self.git_branches, self.test_results, self.lint_results, self.code_reviews, self.quality_gates, self.token_usage, self.correction_attempts, self.activities, self.audit_logs, - self.pull_requests]: + self.pull_requests, self.api_keys]: if repo: repo._async_conn = self._async_conn
🤖 Fix all issues with AI agents
In `@codeframe/auth/api_key_router.py`:
- Around line 87-104: Replace the per-request Database creation in get_db with
the app-scoped singleton: stop using request.state.db and instead return the
shared Database instance stored on request.app.state.db (which is
created/initialized and closed by the server lifespan handler), and adjust
get_api_key_service to call get_db as before; ensure get_db raises a clear error
if request.app.state.db is missing so misconfiguration is obvious.
In `@codeframe/auth/dependencies.py`:
- Around line 160-224: The fallback path in get_api_key_auth creates a
Database(db_path) and calls db.initialize() but never closes it, potentially
leaking connections; modify the function so when request.state.db is None you
either assign the created Database to request.state.db for later cleanup or
ensure the fallback Database is closed after use (e.g., use a try/finally or
context manager pattern around the db usage), add a logger.warning noting the
fallback was used, and ensure db.api_keys.update_last_used and other db accesses
operate on the proper scoped Database instance.
In `@codeframe/persistence/repositories/api_key_repository.py`:
- Around line 20-62: The create() method is storing expires_at as an ISO string
without ensuring UTC, causing lexicographic SQL comparisons in get_by_prefix()
to be incorrect; update create() to normalize any expires_at (and any naive
datetime) to UTC (e.g., convert timezone-aware datetimes with
.astimezone(timezone.utc) and treat naive datetimes as UTC) before calling
.isoformat(), and also ensure the now value in get_by_prefix() is consistently
generated in UTC (datetime.now(timezone.utc)) so both stored expires_at and
comparison timestamps use the same UTC-normalized format.
In `@tests/auth/test_api_key_endpoints.py`:
- Around line 57-68: The fixture defines an unused get_test_db() and a
misleading "Override db dependency" comment; either remove the dead
get_test_db() and update the comment to reflect no override, or actually apply
the override by setting app.dependency_overrides[<dependency>] = get_test_db
before creating TestClient — e.g., in the client fixture assign
app.dependency_overrides[get_db] = get_test_db (or
app.dependency_overrides[get_current_user] if that is the intended dependency)
so the override is active, then create TestClient(app) and clean up the override
after the test.
In `@tests/cli/test_api_key_commands.py`:
- Around line 1-10: Add the module-level pytest v2 marker by defining pytestmark
= pytest.mark.v2 at the top of the test module (ensure pytest is imported);
update the test module that contains the CLI tests (e.g., where CliRunner and
patch are imported) to include this module-level marker so all tests in that
file are marked with pytest.mark.v2.
🧹 Nitpick comments (4)
tests/auth/test_api_keys.py (1)
65-71: Misleading docstring: implementation uses SHA256, not bcrypt.The docstring states "hash should differ (bcrypt uses random salt)" but the actual implementation uses SHA256 which is deterministic. While the test passes because different keys produce different hashes, the explanation is incorrect.
📝 Suggested docstring fix
def test_generate_api_key_hash_uniqueness(self): - """Even with same key, hash should differ (bcrypt uses random salt).""" - # Generate two keys + """Different keys produce different hashes.""" + # Generate two different keys _, hash1, _ = generate_api_key() _, hash2, _ = generate_api_key() # Different keys should have different hashes assert hash1 != hash2codeframe/cli/auth_commands.py (1)
54-69: Database connection is not closed after use.
get_db_for_cli()initializes a database connection that is never explicitly closed. While SQLite handles this gracefully on process exit, it's good practice to close connections, especially if the CLI commands are used in scripts or long-running processes.♻️ Suggested improvement using context manager pattern
Consider returning the database for use in a context manager pattern:
# Option 1: Use context manager in commands def api_key_create(...): db = get_db_for_cli() try: service = ApiKeyService(db) # ... operations finally: db.close() # Option 2: Make get_db_for_cli a context manager from contextlib import contextmanager `@contextmanager` def get_db_for_cli(): db_path = os.getenv(...) db = Database(db_path) db.initialize() try: yield db finally: db.close()tests/auth/test_api_key_endpoints.py (1)
108-131: Test constructs an invalid key that won't match any stored hash.The test
test_create_api_key_requires_jwtcreates a fake API key with the right format but wrong content. This is intentional for testing that the endpoint rejects API key auth, but the comment could clarify that the key verification will fail.Consider adding a brief comment explaining the test relies on the fake key failing verification:
# Construct a fake key with correct format - it will fail hash verification # which is fine since we're testing that API key auth is rejected for creationcodeframe/auth/api_key_router.py (1)
38-43: UseField(default_factory=...)forscopesfor idiomatic clarity.In Pydantic v2, mutable defaults are deep-copied automatically, so shared state isn't an issue. However, using
default_factoryis the idiomatic approach and makes intent explicit.♻️ Suggested refactor
-from pydantic import BaseModel, field_validator +from pydantic import BaseModel, Field, field_validator @@ - scopes: List[str] = [SCOPE_READ, SCOPE_WRITE] + scopes: List[str] = Field(default_factory=lambda: [SCOPE_READ, SCOPE_WRITE])
- Reverse rotate_api_key order: revoke before create (prevents both keys being active if create fails) - Use app-scoped db in api_key_router.py to avoid connection leaks - Add warning for fallback db creation in dependencies.py - Normalize expires_at to UTC in api_key_repository for correct string comparison in expiration checks - Remove unused get_test_db function from test fixture - Add pytest.mark.v2 to CLI test file - Update mock requests in dual_auth tests to include app.state.db
Code Review - API Key Management System (PR #326)Overall AssessmentThis is a well-architected implementation that properly follows the v2 architecture guidelines. The code demonstrates strong adherence to the "core-first, headless" principles outlined in CLAUDE.md. I have identified several areas for improvement around security, error handling, and architectural considerations. ✅ Strengths
🔴 Critical Issues1. Core Module Violates Headless Principle (
|
The client fixture now sets server.app.state.db directly instead of relying on DATABASE_PATH env var. This ensures the test database is used by routes that access request.app.state.db, fixing 'Cannot operate on a closed database' errors when tests run in batch.
Security improvements: - Add expires_at validation: must be in the future if provided - Deduplicate scopes and require at least one scope - Bump failed auth logging from DEBUG to WARNING level Code quality: - Document scope hierarchy in require_scope docstring - Clarify KEY_PREFIX_LENGTH comment (8 prefix + 4 random chars) - Fix rotate_api_key order: create new key first, then revoke old (ensures user always has a working key if creation fails)
Follow-up Review: API Key Authentication System (PR #326)ContextThis PR implements API key authentication for the REST API, which aligns with Phase 2 of the v2 Strategic Roadmap ("Server Layer as Thin Adapter"). This is issue #324 from the roadmap and is appropriate for Phase 2 work. ✅ What's Improved Since Last ReviewGood progress addressing previous concerns:
🎯 Architecture Assessment: Phase 2 AlignmentCritical Question: Does this implementation follow the "thin adapter pattern" for Phase 2? Issue Identified: This PR creates two separate implementations instead of following the Phase 2 pattern:
According to Both should call the same core module. Currently:
📋 Specific Recommendations1. Refactor REST API to Use Core Service (HIGH PRIORITY)Current ( @router.post("", ...)
async def create_api_key(
request: Request,
body: CreateApiKeyRequest,
current_user: User = Depends(get_current_user),
):
db = get_db(request)
# Generate the API key
full_key, key_hash, prefix = generate_api_key()
# Store in database
key_id = db.api_keys.create(
user_id=current_user.id,
name=body.name,
key_hash=key_hash,
prefix=prefix,
scopes=body.scopes,
expires_at=expires_at,
)
# ... build responseShould be (following Phase 2 thin adapter pattern): @router.post("", ...)
async def create_api_key(
request: Request,
body: CreateApiKeyRequest,
current_user: User = Depends(get_current_user),
):
db = get_db(request)
service = ApiKeyService(db) # Use the shared core service
result = service.create_api_key(
user_id=current_user.id,
name=body.name,
scopes=body.scopes,
expires_at=body.expires_at,
)
return CreateApiKeyResponse(
key=result.key,
id=result.id,
prefix=result.prefix,
created_at=result.created_at,
)Apply same pattern to all endpoints: Benefits:
2. Core Module Database Dependency (MEDIUM PRIORITY)
from codeframe.persistence.database import Database
class ApiKeyService:
def __init__(self, db: Database):
self.db = dbObservation: While CLAUDE.md says "core must be headless", many existing v2 core modules ( Recommendation: Keep as-is for consistency with existing v2 modules. If database abstraction becomes a priority, it should be done as a separate refactor across all core modules, not just this one. 3. Security: Missing Validation on
|
| Priority | Issue | Action |
|---|---|---|
| 🔴 HIGH | REST API bypasses core service | Refactor api_key_router.py to use ApiKeyService |
| 🟡 MEDIUM | No expires_at validation |
Add future date validation in create_api_key() |
| 🟡 MEDIUM | Missing cross-layer integration tests | Add tests verifying REST/CLI equivalence |
| 🟢 LOW | Inconsistent type hints | Standardize on list[str] or List[str] |
| 🟢 LOW | Audit logging | Consider structured logging (future enhancement) |
Recommendation
Conditionally approve pending the HIGH priority fix:
The implementation is architecturally sound but needs refactoring to follow the Phase 2 "thin adapter" pattern consistently. The REST API should call ApiKeyService just like the CLI does, rather than reimplementing the logic.
Once the REST endpoints are refactored to use ApiKeyService, this will be an excellent Phase 2 contribution that properly supports the v2 architecture.
Great work on the comprehensive tests and security implementation! 🎉
PR Review: API Key Authentication SystemSummaryThis PR implements a comprehensive API key authentication system for both CLI and REST API, adding dual authentication support (JWT + API keys). The implementation follows the v2 thin adapter pattern appropriately and aligns with Phase 2 deliverables (#324). ✅ StrengthsArchitecture & Design
Code Quality
Security Features
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@codeframe/auth/api_key_router.py`:
- Around line 104-126: The fallback path in get_db creates a Database() and
stores it on request.state.db but there is no middleware to close it, causing
connection leaks; add request-scoped cleanup by implementing middleware that
wraps each request, calls the downstream handler, and in a finally block checks
for hasattr(request.state, "db") and calls the Database.close() (or appropriate
teardown method) for that instance (reference: get_db, request.state.db,
Database.initialize, Database.close), or alternatively replace the fallback with
a context-managed lifecycle hook that creates the DB for the request and ensures
it is closed on teardown.
In `@codeframe/core/api_key_service.py`:
- Around line 62-109: The create_api_key service currently accepts an expires_at
without validation; update the create_api_key method to defensively validate the
expires_at parameter (before calling generate_api_key /
self.db.api_keys.create): if expires_at is provided ensure it is a
timezone-aware datetime and strictly in the future compared to
datetime.now(timezone.utc), otherwise raise ValueError with a clear message;
keep existing behavior when expires_at is None and preserve returning
CreatedApiKey with created_at using datetime.now(timezone.utc).
🧹 Nitpick comments (4)
codeframe/auth/api_keys.py (1)
104-109: Consider lazy-loading the CryptContext to avoid repeated instantiation.Creating a new
CryptContexton every bcrypt verification call adds unnecessary overhead. If bcrypt legacy keys are expected, consider module-level lazy initialization.♻️ Optional improvement
+# Lazy-loaded bcrypt context for legacy hash verification +_pwd_context = None + +def _get_pwd_context(): + global _pwd_context + if _pwd_context is None: + from passlib.context import CryptContext + _pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") + return _pwd_context + def verify_api_key(key: str, key_hash: str) -> bool: ... elif key_hash.startswith("$2"): - from passlib.context import CryptContext - pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") - return pwd_context.verify(key, key_hash) + return _get_pwd_context().verify(key, key_hash)codeframe/auth/dependencies.py (1)
160-163: Request parameter should not default to None.The
request: Request = Nonedefault can cause anAttributeErrorat line 179 (request.app.state) if somehow invoked without a request. FastAPI will always inject the Request, so the default is misleading and hides potential issues.♻️ Suggested fix
async def get_api_key_auth( api_key: Optional[str] = Security(api_key_header), - request: Request = None, + request: Request, ) -> Optional[Dict[str, Any]]:codeframe/core/api_key_service.py (1)
189-211:get_api_keylacks user ownership check.Unlike
revoke_api_key, this method doesn't verify that the requesting user owns the key. While not immediately exploitable if callers always pass the correct context, it could leak key metadata (name, scopes, usage times) if misused.Consider adding a
user_idparameter for ownership verification, consistent with the other methods.♻️ Suggested improvement
- def get_api_key(self, key_id: str) -> Optional[ApiKeyInfo]: + def get_api_key(self, key_id: str, user_id: Optional[int] = None) -> Optional[ApiKeyInfo]: """Get a single API key's info. Args: key_id: The API key ID + user_id: If provided, verify ownership (returns None if not owned) Returns: ApiKeyInfo or None if not found """ key = self.db.api_keys.get_by_id(key_id) if key is None: return None + if user_id is not None and key["user_id"] != user_id: + return None + return ApiKeyInfo( ... )codeframe/auth/api_key_router.py (1)
208-238: Consider preventing self-revocation of the authenticating API key.An API key can currently revoke itself, which might be unexpected behavior. While not strictly a bug, this could lead to confusing scenarios where a client revokes its own key mid-session.
💡 Optional guard
`@router.delete`("/{key_id}", response_model=RevokeApiKeyResponse) async def revoke_api_key( request: Request, key_id: str, auth: dict = Depends(require_auth), ): + # Prevent API key from revoking itself + if auth.get("type") == "api_key" and auth.get("key_id") == key_id: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Cannot revoke the API key being used for authentication", + ) + service = get_api_key_service(request)
| def get_db(request: Request) -> Database: | ||
| """Get database instance from app state (singleton managed by lifespan handler). | ||
|
|
||
| Uses the app-scoped database to avoid per-request connection leaks. | ||
| Falls back to DATABASE_PATH env var if app.state.db not available. | ||
| """ | ||
| # Prefer app-scoped singleton (set by lifespan handler in server.py) | ||
| db = getattr(request.app.state, "db", None) | ||
| if db is not None: | ||
| return db | ||
|
|
||
| # Fallback for tests or standalone usage | ||
| db = getattr(request.state, "db", None) | ||
| if db is None: | ||
| logger.warning("No db in app.state, creating fallback connection") | ||
| db_path = os.getenv( | ||
| "DATABASE_PATH", | ||
| os.path.join(os.getcwd(), ".codeframe", "state.db") | ||
| ) | ||
| db = Database(db_path) | ||
| db.initialize() | ||
| request.state.db = db | ||
| return db |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for middleware and cleanup patterns
rg -n "middleware|@app|lifespan|teardown|close" codeframe --type py -B 2 -A 2 | head -100Repository: frankbria/codeframe
Length of output: 5049
🏁 Script executed:
# Check specifically for request.state.db usage across the codebase
rg -n "request\.state\.db" codeframe --type py -B 2 -A 2Repository: frankbria/codeframe
Length of output: 716
🏁 Script executed:
# Look for Database class and its close/cleanup methods
rg -n "class Database|def close|def __del__|def __exit__" codeframe --type py -B 1 -A 3Repository: frankbria/codeframe
Length of output: 4355
🏁 Script executed:
# Check the server.py file for lifespan handler mentioned in comments
fd "server.py" codeframe --type fRepository: frankbria/codeframe
Length of output: 114
🏁 Script executed:
# Search for any cleanup or context manager patterns in the app
rg -n "async def|@contextmanager|__aenter__|__aexit__|cleanup" codeframe --type py -l | head -20Repository: frankbria/codeframe
Length of output: 715
🏁 Script executed:
# Search for middleware decorators and lifecycle handlers
rg -n "@app\.middleware|@middleware|@request|teardown|startup|shutdown" codeframe --type py -B 1 -A 3Repository: frankbria/codeframe
Length of output: 5826
🏁 Script executed:
# Check dependencies.py fully to understand the middleware cleanup comment
cat codeframe/auth/dependencies.pyRepository: frankbria/codeframe
Length of output: 10231
🏁 Script executed:
# Search for any pattern that cleans up request state after request ends
rg -n "request\.state|\.state\." codeframe --type py -B 2 -A 2 | head -150Repository: frankbria/codeframe
Length of output: 7820
Fallback database connections are not cleaned up—middleware mentioned in code comment doesn't exist.
The fallback path creates a database connection and stores it on request.state.db with a comment stating it "can be cleaned up by middleware," but no such middleware exists in the codebase. Only CORSMiddleware is configured. This means any request that triggers the fallback path will leak the database connection when the request ends.
Either implement middleware to explicitly close request.state.db after each request, or use a context manager pattern to guarantee cleanup (e.g., wrap the fallback db creation in a request lifecycle hook that closes the connection on teardown).
🤖 Prompt for AI Agents
In `@codeframe/auth/api_key_router.py` around lines 104 - 126, The fallback path
in get_db creates a Database() and stores it on request.state.db but there is no
middleware to close it, causing connection leaks; add request-scoped cleanup by
implementing middleware that wraps each request, calls the downstream handler,
and in a finally block checks for hasattr(request.state, "db") and calls the
Database.close() (or appropriate teardown method) for that instance (reference:
get_db, request.state.db, Database.initialize, Database.close), or alternatively
replace the fallback with a context-managed lifecycle hook that creates the DB
for the request and ensures it is closed on teardown.
- Update status badge to Phase 2 In Progress (4285 tests) - Add API key authentication section with CLI commands - Add rate limiting configuration documentation - Document new V2 API endpoints - Add Security & API section to Key Features - Update roadmap to show Phase 2 at 90% complete - Mark #322, #325, #326, #327 as complete - Update current focus to remaining items (WebSocket, OpenAPI, pagination)
1 participant
Summary
Changes
Core Service Layer
codeframe/core/api_key_service.py- Shared business logic for create/list/revoke/rotateREST API
POST /api/auth/api-keys- Create API key (JWT auth required)GET /api/auth/api-keys- List user's keys (JWT or API key auth)DELETE /api/auth/api-keys/{id}- Revoke key (JWT or API key auth)CLI Commands
cf auth api-key-create --name "..." --user-id N [--scopes ...]cf auth api-key-list --user-id Ncf auth api-key-revoke <id> --user-id N [--yes]cf auth api-key-rotate <id> --user-id NSecurity Features
Test plan
Summary by CodeRabbit
New Features
CLI
Persistence
Tests