fix: #118 #120
Merged
fix: #118 #120
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughReplaces environment-variable (.env) configuration with layered TOML files (config.toml, secrets.toml, per-service overrides). Refactors Settings to load from TOML (BaseModel), updates Docker/compose/CI to copy/mount TOML files, and changes workers/tests to pass config/override paths. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI
participant Image as Docker Image
participant Container as Container Runtime
participant Settings as Settings loader
participant Service as Worker/API
CI->>Image: copy `backend/config.test.toml` & `backend/secrets.toml`
Image->>Container: start container (mount config/*.toml & secrets.toml)
Container->>Settings: Settings(config_path="config.toml", secrets_path="secrets.toml", override_path="config.service.toml")
Settings-->>Container: return merged configuration
Container->>Service: start service with resolved settings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 31 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/tests/load/config.py">
<violation number="1" location="backend/tests/load/config.py:10">
P2: Switching LoadConfig to BaseModel drops BaseSettings environment loading, so documented LOAD_* env overrides stop working (e.g., LOAD_BASE_URL). This is a functional regression for load test configuration.</violation>
</file>
<file name="backend/config.test.toml">
<violation number="1" location="backend/config.test.toml:6">
P1: Avoid committing secrets or passwords in config. Move SECRET_KEY and MongoDB credentials to environment variables or a secrets manager, and reference them here instead.</violation>
</file>
<file name="backend/config.toml">
<violation number="1" location="backend/config.toml:6">
P0: Hard-coded SECRET_KEY committed to the repo. Secrets should be provided via environment/secret management, not stored in versioned config.</violation>
<violation number="2" location="backend/config.toml:10">
P1: Plaintext MongoDB credentials are committed in config. Use environment/secret management and avoid root credentials in repo.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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
🤖 Fix all issues with AI agents
In `@backend/config.toml`:
- Line 10: The MONGODB_URL entry currently contains hardcoded credentials
("root:rootpassword"); remove the secret from the value and replace it with a
non-secret default or placeholder (e.g., a connection string that references
environment variables) and add a short comment in the config explaining this
file is for development defaults only and production must override via
environment variables; specifically update the MONGODB_URL setting to not embed
credentials and document how to supply real credentials (env var names) so
callers of MONGODB_URL (e.g., code that reads that config) will use secure,
overridable secrets in production.
- Line 6: Remove the hardcoded SECRET_KEY entry from the tracked config (the
SECRET_KEY setting in backend/config.toml) and migrate secret handling to a
secure source: add support for loading SECRET_KEY from an environment variable
(e.g., process.env.SECRET_KEY or the app's config loader) and update the config
loader to fall back to a gitignored config.local.toml for local overrides;
alternatively wire the loader to a secrets manager. Ensure you update README or
example config to show a placeholder SECRET_KEY and add config.local.toml to
.gitignore so secrets are never committed.
🧹 Nitpick comments (4)
backend/config.test.toml (1)
1-8: Acknowledge thatSECRET_KEYis intentionally test-only.Gitleaks flagged this as a potential secret exposure. While this is a test configuration file and the key is clearly intended for local/CI testing only, consider adding a comment to make this explicit and prevent future false positives:
# Integr8sCode backend test configuration # Differences from config.toml: lower timeouts, faster bcrypt, relaxed rate limits +# NOTE: SECRET_KEY below is for test/CI use only - never use in production PROJECT_NAME = "integr8scode" DATABASE_NAME = "integr8scode_db" SECRET_KEY = "uS5xBF-OKXHV-1vqU4ASLwyPcKpSdUTLqGHPYs3y-Yc"This documents intent and helps security scanners (and reviewers) understand it's safe.
backend/Dockerfile (1)
29-42: Gunicorn parameters are hardcoded, ignoringWEB_*settings from config.toml.The entrypoint script uses hardcoded values (
--workers 4,--threads 4,--timeout 60,--backlog 2048) instead of reading from the TOML configuration. TheWEB_CONCURRENCY,WEB_THREADS,WEB_TIMEOUT, andWEB_BACKLOGfields inconfig.tomlandSettingsare now effectively unused for the main backend service.Consider either:
- Removing the
WEB_*fields from Settings/config.toml if they're not needed- Or using a wrapper script that reads the TOML and passes values to Gunicorn
backend/config.toml (1)
50-50: 100% tracing sampling rate may impact production performance.
TRACING_SAMPLING_RATE = 1.0samples all requests, which can significantly impact performance and storage costs in production. The Settings class default is0.1(10%). Consider using a lower rate for production defaults.backend/app/settings.py (1)
24-30: Consider adding user-friendly error handling for missing config files.If the config file doesn't exist,
FileNotFoundErrorpropagates with a generic message. A clearer error message would help with debugging deployment issues.♻️ Suggested improvement
def __init__(self, config_path: str = "config.toml", override_path: str | None = None) -> None: - with open(config_path, "rb") as f: - data = tomllib.load(f) + try: + with open(config_path, "rb") as f: + data = tomllib.load(f) + except FileNotFoundError: + raise FileNotFoundError(f"Configuration file not found: {config_path}") if override_path: - with open(override_path, "rb") as f: - data |= tomllib.load(f) + try: + with open(override_path, "rb") as f: + data |= tomllib.load(f) + except FileNotFoundError: + raise FileNotFoundError(f"Override configuration file not found: {override_path}") super().__init__(**data)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/app/settings.py`:
- Around line 36-44: The code opens config_path and override_path without
validating existence and only secrets_path uses Path(secrets_path).is_file();
update the block that builds data so all three paths are checked consistently:
verify Path(config_path).is_file() before opening and raise a clear, descriptive
exception if missing (including the path and context), use the existing Pattern
(Path(...).is_file()) for override_path before attempting to open it, and
preserve merging with tomllib.load and the final super().__init__(**data); also
consider catching tomllib errors to wrap them with a helpful message referencing
the failing path.
In `@docker-compose.yaml`:
- Around line 109-111: Add a secrets example and documentation because
docker-compose.yaml mounts ./backend/secrets.toml (used by services in the
compose file) but that file is gitignored and absent in fresh clones; create a
new backend/secrets.example.toml containing all required keys/placeholder values
and short comments, add backend/secrets.toml to .gitignore if not already, and
update README/CONTRIBUTING to document copying secrets.example.toml to
backend/secrets.toml (and any required env values) so docker-compose up won’t
fail on missing mounts.
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/secrets.example.toml">
<violation number="1" location="backend/secrets.example.toml:16">
P1: Do not commit real-looking secrets or credentials in the example secrets file. Keep placeholder values so teams are forced to supply their own secrets.</violation>
<violation number="2" location="backend/secrets.example.toml:17">
P1: Replace the example MongoDB URL with a placeholder (no real credentials) to avoid committing reusable passwords.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/config.test.toml`:
- Around line 1-4: The comment in backend/config.test.toml incorrectly
references "secrets.test.toml" while CI actually uses the repository's secrets
file; either create and populate a dedicated secrets.test.toml or update the
comment to reference the real CI secrets filename (change "secrets.test.toml" to
"secrets.toml" or the correct secrets filename) so the comment matches the CI
workflow; edit the header comment in backend/config.test.toml accordingly.
🧹 Nitpick comments (1)
docs/reference/configuration.md (1)
36-38: Consider using named anchors instead of line numbers for snippets.The snippets reference specific line ranges (e.g.,
backend/config.toml:1:9). Ifconfig.tomlis modified, these line numbers may become stale, causing the documentation to show incorrect content. Consider using named anchors in the TOML file or documenting this fragility for maintainers.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Replaces .env/pydantic-settings with TOML-based config loaded via tomllib, with per-worker overrides and a secrets.toml layer. Fixes #118 by simplifying config management and making settings consistent across dev, CI, and runtime.
Refactors
Migration
Written for commit bbf99d9. Summary will update on new commits.
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.