fix: enforce mandatory API key in production (fail-closed)#104
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements fail-closed API key authentication for production deployments, adding environment detection and startup/runtime guards in the API server, updating docs and ChangesFail-Closed Authentication in Production
Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI_App as FastAPI App
participant AuthDep as _require_api_key
participant Env as Environment (AGENTWATCH_ENV / AGENTWATCH_API_KEY)
Client->>FastAPI_App: request to protected endpoint (X-Api-Key header)
FastAPI_App->>AuthDep: invoke dependency
AuthDep->>Env: read AGENTWATCH_ENV / AGENTWATCH_API_KEY
alt production && API key missing
AuthDep-->>FastAPI_App: raise HTTP 500 (Server misconfiguration)
else API key configured
AuthDep-->>FastAPI_App: validate header -> 401 or allow
end
note over FastAPI_App,Env: On startup lifespan checks Env and raises RuntimeError if production and key missing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
Comment |
…maintaining production fail-closed requirement
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@agentwatch/api/server.py`:
- Around line 355-360: When running in production, do not merely log a critical
error; fail startup when AGENTWATCH_API_KEY is missing by raising an exception
or exiting the process in the check that uses _IS_PROD and _API_KEY. Modify the
block referencing _IS_PROD, _API_KEY and logger (in server.py) so that after
logging the critical message you raise a RuntimeError (or call sys.exit(1)) with
a clear message like "AGENTWATCH_API_KEY is required in production" to stop the
server from starting and ensure the _require_api_key dependency is not relied on
to surface the misconfiguration later.
- Around line 175-176: The code currently defaults _ENV to "production" when
AGENTWATCH_ENV/ENVIRONMENT is unset which forces prod behavior; change the
default to a non-production value (e.g., "development" or empty string) when
computing _ENV and keep _IS_PROD = _ENV.lower() == "production" so
production-only behavior only occurs if the env var explicitly equals
"production"; update the _ENV/_IS_PROD initialization (references: _ENV and
_IS_PROD) accordingly to avoid implicit production defaults.
In `@docs/getting-started.md`:
- Around line 214-221: The docs show a fail-closed production rule but the
earlier defaults snippet leaves AGENTWATCH_ENV=production without an API key,
causing protected endpoints to 500; update the earlier defaults snippet so it
either sets AGENTWATCH_ENV=development or provides an explicit
AGENTWATCH_API_KEY value (or a placeholder like YOUR_DEV_KEY) to avoid
misconfigured production behavior; ensure the change references the same
environment variables AGENTWATCH_ENV and AGENTWATCH_API_KEY so readers see
consistent quickstart defaults and the Security & Authentication section aligns.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17666e8d-8218-4f31-b716-f8ac7a589ede
📒 Files selected for processing (3)
.env.exampleagentwatch/api/server.pydocs/getting-started.md
…API key and ensure docs consistency
|
@sreerevanth
It looks like the tests run, but the action fails when trying to post the PR summary comment. Since this PR comes from a fork, I think the |
Adds issues: write permission required to post comments on pull requests.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test-on-pr.yml (1)
18-19: 💤 Low valueConsider disabling credential persistence.
Static analysis flags that
persist-credentialsdefaults totrue, which stores the Git token in.git/config. While this workflow doesn't upload artifacts, explicitly settingpersist-credentials: falseis a defense-in-depth measure if artifacts are added later.🔒 Optional hardening
- name: Checkout uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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 @.github/workflows/test-on-pr.yml around lines 18 - 19, Update the Checkout step that uses actions/checkout@v4 to explicitly disable credential persistence by adding persist-credentials: false to the step configuration (the step with name: Checkout and uses: actions/checkout@v4) so the GitHub token is not stored in .git/config as a defense-in-depth measure.
🤖 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 @.github/workflows/test-on-pr.yml:
- Around line 7-11: The workflow uses on: pull_request and requests issues:
write / pull-requests: write but GitHub makes GITHUB_TOKEN read-only for forked
PRs, causing actions/github-script steps that call
issues.createComment/updateComment to 403; fix by either enabling the repo
setting "Send write tokens to workflows from fork pull requests", or change the
workflow trigger to pull_request_target and ensure you do NOT check out or run
untrusted code from the forked head (keep any checkout/use of PR code guarded),
or restrict the workflow to same-repo PRs only (keep on: pull_request but accept
write actions will fail for forks). Update the permissions block and any
documentation/comments in the workflow accordingly and ensure the
actions/github-script step references are adjusted to the chosen approach.
---
Nitpick comments:
In @.github/workflows/test-on-pr.yml:
- Around line 18-19: Update the Checkout step that uses actions/checkout@v4 to
explicitly disable credential persistence by adding persist-credentials: false
to the step configuration (the step with name: Checkout and uses:
actions/checkout@v4) so the GitHub token is not stored in .git/config as a
defense-in-depth measure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e14aac9c-602f-4d62-bb3f-4e92b8ba6e00
📒 Files selected for processing (1)
.github/workflows/test-on-pr.yml
This commit switches the PR test workflow to pull_request_target to allow status comments on forked PRs, while adding a security guard to prevent untrusted code from accessing repo secrets. It also includes general style and lint cleanups across the test suite.
🧪 PR Test Results
Python 3.12 · commit 7ab41b4 |
This commit applies missing # noqa suppressions to test files and fixes import sorting/formatting in tests/test_models.py, ensuring the CI lint check passes.
sreerevanth
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
This improves the default security posture by enforcing fail-closed behavior in production while preserving a smooth local development experience. The workflow hardening and documentation updates are also appreciated.
Approved.
Hardens API authentication to ensure a fail-closed posture in production environments when AGENTWATCH_API_KEY is missing. This fix ensures that production deployments refuse unauthorized access by default if misconfigured. Closes #103
Summary by CodeRabbit
Bug Fixes
Documentation
Chores