Nix support #371
Nix support #371
Conversation
WalkthroughAdds a Nix-based reproducible deployment and development stack plus a container deployment guide: new Nix flake, NixOS module, overlay, package derivation, microVM config, NixOS test, missing-hashes manifest, and extended deployment documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin
participant NixFlake
participant NixOS
participant Systemd
participant SourceBot
participant PostgreSQL
participant Redis
Admin->>NixFlake: build/apply flake (module, package)
NixFlake->>NixOS: produce configuration
NixOS->>Systemd: enable/start units (web, backend, zoekt)
Systemd->>SourceBot: launch services
SourceBot->>PostgreSQL: initialize/connect DB
SourceBot->>Redis: connect/cache setup
SourceBot->>Admin: expose API/web on configured port
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 8
🔭 Outside diff range comments (1)
nix/missing-hashes.json (1)
1-181: Improve automated hash verificationThe current suggestion uses
dist.shasum(SHA-1) from the npm registry, which won’t match the SHA-256 entries innix/missing-hashes.json. Instead, fetch each package tarball and compute its SHA-256. For example:#!/usr/bin/env bash # Verify SHA-256 for a sample of packages against missing-hashes.json # Sample list of entries <name>@<version> packages=( "@esbuild/linux-x64@0.25.1" "@next/swc-darwin-arm64@14.2.26" ) for pkg in "${packages[@]}"; do name=${pkg%@*} version=${pkg#*@} # Fetch tarball URL url=$(curl -s "https://registry.npmjs.org/$name/$version" | jq -r '.dist.tarball') echo "Verifying $pkg from $url" # Compute SHA-256 (Nix’s prefetch-url is ideal here) actual=$(nix-prefetch-url --unpack --type sha256 "$url" 2>/dev/null) expected=$(jq -r --arg key "${name}@npm:${version}" '.[$key]' nix/missing-hashes.json) echo " expected: $expected" echo " actual: $actual" if [[ "$actual" == "$expected" ]]; then echo " ✅ OK" else echo " ❌ MISMATCH" exit 1 fi done• No changes needed in
nix/missing-hashes.jsonitself.
• Incorporate this (or similar) check into your CI to catch stale or incorrect hashes automatically.
🧹 Nitpick comments (7)
nix/overlay.nix (1)
1-12: Well-structured overlay with appropriate package additions.The overlay correctly adds the sourcebot package and updates zoekt to a specific revision. The structure follows Nix overlay best practices.
Consider adding comments to clarify the purpose of the zoekt override:
final: prev: { sourcebot = final.callPackage ./sourcebot.nix {}; + # Override zoekt with updated version compatible with sourcebot zoekt = prev.zoekt.overrideAttrs (old: rec { vendorHash = "sha256-laiBp+nMWEGofu7zOgfM2b8MIC+Dfw7eCLgb/5zf9oo="; src = final.fetchFromGitHub { owner = "sourcegraph"; repo = "zoekt"; + # Updated to revision with sourcebot compatibility improvements rev = "12a2f4ad075359a09bd8a91793acb002211217aa"; hash = "sha256-JByTgJsnqLlP7hNbQumM4zqZZuj7igc2V35vw0ahCqM="; }; }); }Note: The static analysis warning about "sourcegraph-access-token" is a false positive - this is just the repository owner name in the GitHub URL.
docs/docs/deployment-guide.mdx (1)
101-208: Comprehensive NixOS deployment documentation with good examples.The documentation provides clear step-by-step instructions for deploying SourceBot on NixOS, including proper secret management and configuration examples.
Consider these improvements for better security and usability:
- Reduce concurrency values in the example - The current values (1000) are extremely high:
settings = { resyncConnectionIntervalMs = 1000 * 60 * 60 * 24 * 7; # 1 week reindexIntervalMs = 1000 * 60 * 60 * 24 * 7; # 1 week - maxRepoIndexingJobConcurrency = 1000; # 8 default - maxConnectionSyncJobConcurrency = 1000; # 8 default - maxRepoGarbageCollectionJobConcurrency = 1000; # 8 default + maxRepoIndexingJobConcurrency = 8; # Default value + maxConnectionSyncJobConcurrency = 8; # Default value + maxRepoGarbageCollectionJobConcurrency = 8; # Default value };
- Add security note about file permissions:
+ <Warning> + Ensure your secrets files have restrictive permissions (600) and are owned by the sourcebot user. + </Warning>
- Add firewall configuration guidance:
+ <Step title="Configure firewall (optional)"> + If you need to access SourceBot from external networks, configure your firewall: + + ```nix + networking.firewall.allowedTCPPorts = [ 7734 ]; # Default SourceBot port + ``` + </Step>nix/nixosTest.nix (1)
1-31: Good basic test structure for NixOS module validation.The test correctly validates that all essential SourceBot services start up and respond to HTTP requests.
Consider enhancing the test for better coverage and robustness:
testScript = '' start_all() machine.wait_for_unit("sourcebot-web.service") machine.wait_for_unit("sourcebot-backend.service") machine.wait_for_unit("sourcebot-db-setup.service") machine.wait_for_unit("postgresql.service") machine.wait_for_unit("sourcebot-zoekt.service") - machine.succeed("${pkgs.curl}/bin/curl http://localhost:7734") + + # Wait for web service to be ready + machine.wait_for_open_port(7734) + + # Test web interface responds correctly + machine.succeed("${pkgs.curl}/bin/curl -f http://localhost:7734") + + # Verify database connection + machine.succeed("systemctl is-active postgresql.service") + + # Check logs for any startup errors + machine.succeed("! journalctl -u sourcebot-web.service --since=-1m | grep -i error") '';Also consider using a minimal but valid configuration instead of empty JSON:
configPath = "${pkgs.writeText "config" '' - { - } + { + "$schema": "https://raw.githubusercontent.com/sourcebot-dev/sourcebot/main/schemas/v3/index.json", + "connections": {} + } ''}"nix/microvm.nix (1)
40-43: Consider security implications of dev environment settings.The empty root password and autologin are appropriate for development but should be documented as such.
+ # Development environment settings - NOT for production users.users.root.password = ""; # Enable autologin for root user services.getty.autologinUser = "root";nix/nixosModule.nix (2)
52-52: Fix typo in option description- description = "Open Firwall ports for SourceBot"; + description = "Open Firewall ports for SourceBot";
64-68: Consider security implications of default hostnameThe default hostname
0.0.0.0binds to all network interfaces, which might expose the service more broadly than intended. Consider using127.0.0.1as default for better security.flake.nix (1)
15-15: Consider supporting additional architecturesThe flake is currently limited to
x86_64-linux. Consider supporting additional architectures likeaarch64-linuxfor broader compatibility.- flake-utils.lib.eachSystemPassThrough ["x86_64-linux"] (system: { + flake-utils.lib.eachSystemPassThrough ["x86_64-linux" "aarch64-linux"] (system: {and
- // flake-utils.lib.eachSystem ["x86_64-linux"] ( + // flake-utils.lib.eachSystem ["x86_64-linux" "aarch64-linux"] (Also applies to: 30-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
docs/docs/deployment-guide.mdx(3 hunks)flake.nix(1 hunks)nix/microvm.nix(1 hunks)nix/missing-hashes.json(1 hunks)nix/nixosModule.nix(1 hunks)nix/nixosTest.nix(1 hunks)nix/overlay.nix(1 hunks)nix/sourcebot.nix(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.26.0)
nix/overlay.nix
8-8: Sourcegraph is a code search and navigation engine.
(sourcegraph-access-token)
🔇 Additional comments (2)
nix/microvm.nix (1)
45-62: Well-configured SourceBot service for development.The service configuration is appropriate for development with debug logging and a comprehensive test configuration including both GitHub and the JSON schema reference.
nix/nixosModule.nix (1)
191-191: Verify different restart policies for servicesThe web service uses
Restart = "always"while the backend service usesRestart = "on-failure". Is this intentional?Also applies to: 224-224
| runHook preInstall | ||
|
|
||
| cp -r packages/web/.next/standalone $out | ||
| cp -r node_modules/* $out/node_modules |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant node_modules copying
The node_modules directory is being copied multiple times which could cause conflicts and is inefficient. Consider consolidating these operations.
- cp -r node_modules/* $out/node_modules
-
mkdir -p $out/packages/web
cp -r packages/web/public $out/packages/web/public
mkdir -p $out/packages/web/.next
cp -r packages/web/.next/static $out/packages/web/.next/static
mkdir -p $out/packages/backend
- cp -r packages/backend/node_modules/* $out/node_modules
cp -r packages/backend/* $out/packages/backend
cp -r packages/db/* $out/packages/db
cp -r packages/schemas/* $out/packages/schemas
cp -r packages/crypto/* $out/packages/crypto
cp -r packages/error $out/packages/error
cp -r packages/mcp $out/packages/mcp
cp -r node_modules/* $out/node_modulesAlso applies to: 57-57, 65-65
🤖 Prompt for AI Agents
In nix/sourcebot.nix at lines 49, 57, and 65, the node_modules directory is
copied multiple times redundantly. Remove the duplicate cp -r node_modules/*
$out/node_modules commands at lines 57 and 65, keeping only one copy operation
to avoid conflicts and improve efficiency.
|
|
||
| mkdir -p $out/bin | ||
|
|
||
| rm -rf $out/pacakages/web/.next/cache |
There was a problem hiding this comment.
Fix typo in directory path
There's a typo in the path: pacakages should be packages.
Apply this fix:
- rm -rf $out/pacakages/web/.next/cache
+ rm -rf $out/packages/web/.next/cache📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rm -rf $out/pacakages/web/.next/cache | |
| rm -rf $out/packages/web/.next/cache |
🤖 Prompt for AI Agents
In nix/sourcebot.nix at line 69, correct the typo in the directory path by
changing "pacakages" to "packages" in the rm command to ensure the correct cache
directory is targeted for removal.
| ++ optional (cfg.envFile == null) [ | ||
| "AUTH_SECRET=00000000000000000000000000000000000000000000" | ||
| "SOURCEBOT_ENCRYPTION_KEY=00000000000000000000000000000000" | ||
| ]; |
There was a problem hiding this comment.
Avoid hardcoded secrets in environment
Using hardcoded placeholder secrets even as defaults is a security risk. Consider requiring users to provide an envFile or generate random secrets.
Consider either:
- Making
envFilerequired when auth is enabled - Generating random secrets at runtime
- Providing clear documentation that these are insecure defaults
Would you like me to implement a solution that generates secure random secrets?
Also applies to: 218-221
🤖 Prompt for AI Agents
In nix/nixosModule.nix around lines 185 to 188 and also lines 218 to 221, the
code uses hardcoded placeholder secrets for AUTH_SECRET and
SOURCEBOT_ENCRYPTION_KEY, which is a security risk. To fix this, remove the
hardcoded secrets and instead require users to provide an envFile when
authentication is enabled or implement logic to generate secure random secrets
at runtime. Additionally, update documentation to clearly state that default
secrets are insecure if you keep any defaults.
| configPath = mkOption { | ||
| type = types.str; | ||
| description = "Path to the SourceBot configuration file"; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add default value for configPath
The configPath option lacks a default value, which could cause runtime errors if not explicitly set by users.
Consider adding a sensible default:
configPath = mkOption {
type = types.str;
+ default = "${cfg.dataDir}/config.json";
description = "Path to the SourceBot configuration file";
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| configPath = mkOption { | |
| type = types.str; | |
| description = "Path to the SourceBot configuration file"; | |
| }; | |
| configPath = mkOption { | |
| type = types.str; | |
| default = "${cfg.dataDir}/config.json"; | |
| description = "Path to the SourceBot configuration file"; | |
| }; |
🤖 Prompt for AI Agents
In nix/nixosModule.nix around lines 55 to 58, the configPath option is missing a
default value, which may lead to runtime errors if users do not set it. Add a
default value field to the mkOption definition, providing a sensible default
path string for the SourceBot configuration file to ensure safe fallback
behavior.
| ExecStart = "${pkgs.prisma}/bin/prisma migrate deploy --schema ${cfg.package}/packages/db/prisma/schema.prisma"; | ||
| Environment = [ | ||
| "DATABASE_URL=postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot" | ||
| "PATH=${makeBinPath (with pkgs; [prisma openssl])}" |
There was a problem hiding this comment.
Import makeBinPath function
The makeBinPath function is used but not imported.
- "PATH=${makeBinPath (with pkgs; [prisma openssl])}"
+ "PATH=${lib.makeBinPath (with pkgs; [prisma openssl])}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "PATH=${makeBinPath (with pkgs; [prisma openssl])}" | |
| "PATH=${lib.makeBinPath (with pkgs; [prisma openssl])}" |
🤖 Prompt for AI Agents
In nix/nixosModule.nix at line 154, the function makeBinPath is used but not
imported. To fix this, import makeBinPath from the appropriate package or module
at the beginning of the file so it is available for use in constructing the PATH
variable.
|
@msukkari , if you are intrested in this change, I would fix coderabbitai suggestions. But also I recommend to somehow run Nix in CI. Basically these commands:
Sometimes you would need to update the Another option is to push this change to nixpkgs main repository, then any NixOS user would be able to deploy Sourcebot, but you would have less control over its deployment options and it depends on contributors (or you) to maintain its version. |
msukkari
left a comment
msukkari
left a comment
There was a problem hiding this comment.
Thanks for this change @abraverm! Just left a few comments on the docs page to fix broken links and styling
Adding the build to CI shouldn't be too hard and is something we can look into once we get some bandwidth. I think we should be safe to merge this for now and add CI once usage of this deployment flow increases?
docs/docs/deployment-guide.mdx
Outdated
| ]; | ||
| } | ||
| ``` | ||
| [Learn more about NixOS flakes](/docs/installation/nixos-flakes). |
There was a problem hiding this comment.
is this meant to link to external docs? It seems this is broken since no nixos-flakes docs page exists
docs/docs/deployment-guide.mdx
Outdated
| </Step> | ||
| <Step title="Enable Sourcebot"> | ||
| The following NixOS configuration will enable Sourcebot and set it up to run with the provided configuration. | ||
| Additional options could be found in the [source file](../../nix/nixosModule.nix) |
There was a problem hiding this comment.
It seems this is meant to link to the nixosModule.nix file in the repo, in which case you have to link to the file itself on GitHub. Mintlify (the docs library we're using) doesn't support linking to files outside of the docs folder like this as far as I'm aware
docs/docs/deployment-guide.mdx
Outdated
|
|
||
| - [sops-nix](https://github.com/Mic92/sops-nix) example: | ||
|
|
||
| ```nix |
There was a problem hiding this comment.
nit: please add additional params to the code snippets to follow the styling of the rest of the document
For example:
bash wrap icon="terminal" Create example config
The icon can be any valid lucide icon: https://lucide.dev/icons/
For more info check out the mintlify docs: https://mintlify.com/docs/code#code-block-options
| })}"; | ||
| }; | ||
| ``` | ||
| </Step> |
There was a problem hiding this comment.
nit: please add the Login and Done steps from the container deployment here. Copy and paste should be fine!
|
Also don't feel like you need to address all of the coderabbit comments! It sometimes provides reviews that aren't really needed |
nix/sourcebot.nix
Outdated
| missingHashes = ./missing-hashes.json; | ||
| offlineCache = pkgs.yarn-berry.fetchYarnBerryDeps { | ||
| inherit (finalAttrs) src missingHashes; | ||
| hash = "sha256-mM268PS5jgT+glNRtjNMLV936Z/Xgd16YavADWUiSmE="; |
There was a problem hiding this comment.
@msukkari , this line would need to be updated every commit or release: https://github.com/NixOS/nixpkgs/blob/master/doc%2Flanguages-frameworks%2Fjavascript.section.md#yarn-berry_xfetchyarnberrydeps-javascript-fetchyarnberrydeps
I suggest to go with release based update, and I will update the doc on how to pick specific version.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nix/overlay.nix (1)
3-11: Zoekt override looks correct, but note maintenance requirements.The
zoektpackage override is properly structured with updatedvendorHashand source. However, be aware that the pinned revision and hashes will need periodic updates to stay current with upstream changes.Consider documenting the process for updating these values in your contribution guidelines, as mentioned in the PR comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
docs/docs/deployment-guide.mdx(3 hunks)flake.nix(1 hunks)nix/microvm.nix(1 hunks)nix/missing-hashes.json(1 hunks)nix/nixosModule.nix(1 hunks)nix/nixosTest.nix(1 hunks)nix/overlay.nix(1 hunks)nix/sourcebot.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/docs/deployment-guide.mdx
- nix/nixosTest.nix
- nix/missing-hashes.json
- nix/microvm.nix
- flake.nix
- nix/nixosModule.nix
- nix/sourcebot.nix
🧰 Additional context used
🪛 Gitleaks (8.26.0)
nix/overlay.nix
8-8: Sourcegraph is a code search and navigation engine.
(sourcegraph-access-token)
🔇 Additional comments (2)
nix/overlay.nix (2)
1-2: LGTM! Well-structured overlay with proper package definition.The overlay structure follows Nix conventions correctly, and the
sourcebotpackage definition usingcallPackageis appropriate.
8-8: False positive: This is a git revision hash, not an access token.The static analysis tool incorrectly flagged this as a sourcegraph-access-token. Line 8 contains a git revision hash (
12a2f4ad075359a09bd8a91793acb002211217aa), which is perfectly safe and expected in Nix expressions.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
nix/nixosModule.nix (5)
20-20: Fix incorrect package descriptionThis issue was previously flagged but remains unaddressed. The description references "nixos-cli" instead of "sourcebot".
55-58: Add default value for configPathThis issue was previously flagged but remains unaddressed. The configPath option needs a default value to prevent runtime errors.
159-159: Import makeBinPath functionThis issue was previously flagged but remains unaddressed. The makeBinPath function needs to be properly imported.
191-194: Avoid hardcoded secrets in environmentThis security issue was previously flagged but remains unaddressed. The hardcoded placeholder secrets pose a security risk.
225-228: Avoid hardcoded secrets in environmentThis security issue was previously flagged but remains unaddressed. The hardcoded placeholder secrets pose a security risk.
🧹 Nitpick comments (1)
nix/nixosModule.nix (1)
52-52: Fix typo in firewall option description- description = "Open Firwall ports for SourceBot"; + description = "Open Firewall ports for SourceBot";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nix/nixosModule.nix(1 hunks)nix/sourcebot.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- nix/sourcebot.nix
🧰 Additional context used
🧠 Learnings (1)
nix/nixosModule.nix (1)
Learnt from: drew-u410
PR: sourcebot-dev/sourcebot#328
File: CONTRIBUTING.md:5-5
Timestamp: 2025-06-03T00:22:00.498Z
Learning: In the sourcebot repository CONTRIBUTING.md file, the maintainer prefers using "NodeJS" spelling instead of the official "Node.js" branding, and prefers not to add alt text to favicon images in the installation instructions.
| "SOURCEBOT_AUTH_ENABLED=${boolToString cfg.authEnabled}" | ||
| "SOURCEBOT_TELEMETRY_DISABLED=${boolToString cfg.telemetryDisabled}" | ||
| "SOURCEBOT_PUBLIC_KEY_PATH=${cfg.package}/public.pem" | ||
| "AUTH_URL=http://${cfg.hostname}:${toString cfg.port}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent authUrl reference
The backend service manually constructs the AUTH_URL instead of using the configured cfg.authUrl value, creating inconsistency.
- "AUTH_URL=http://${cfg.hostname}:${toString cfg.port}"
+ "AUTH_URL=${cfg.authUrl}"🤖 Prompt for AI Agents
In nix/nixosModule.nix at line 223, replace the manual construction of AUTH_URL
using cfg.hostname and cfg.port with the configured cfg.authUrl value to ensure
consistency. Use cfg.authUrl directly in the string instead of building the URL
manually.
| authUrl = mkOption { | ||
| type = types.str; | ||
| default = "http://${cfg.hostname}:${toString cfg.port}"; | ||
| description = "Hostname or IP address for the SourceBot web server to bind to"; | ||
| }; |
There was a problem hiding this comment.
Fix incorrect authUrl description and default value
The description is copied from the hostname option and doesn't match the purpose. Additionally, the string interpolation in the default value won't work properly in Nix.
- authUrl = mkOption {
- type = types.str;
- default = "http://${cfg.hostname}:${toString cfg.port}";
- description = "Hostname or IP address for the SourceBot web server to bind to";
- };
+ authUrl = mkOption {
+ type = types.str;
+ default = "http://localhost:${toString cfg.port}";
+ description = "Base URL for authentication callbacks";
+ };🤖 Prompt for AI Agents
In nix/nixosModule.nix around lines 69 to 73, the authUrl option has an
incorrect description that does not reflect its purpose and uses string
interpolation that is not valid in Nix. Update the description to accurately
describe authUrl's role, and replace the default value with a proper Nix
expression that concatenates strings and converts values correctly without using
${} interpolation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
nix/nixosModule.nix (6)
20-20: Fix incorrect package description.The description incorrectly references "nixos-cli" instead of "sourcebot".
- description = "Package to use for nixos-cli"; + description = "Package to use for sourcebot";
55-58: Add default value for configPath.The
configPathoption lacks a default value, which could cause runtime errors if not explicitly set by users.Consider adding a sensible default:
configPath = mkOption { type = types.str; + default = "${cfg.dataDir}/config.json"; description = "Path to the SourceBot configuration file"; };
69-73: Fix incorrect authUrl description and default value.The description is copied from the hostname option and doesn't match the purpose. Additionally, the string interpolation in the default value won't work properly in Nix.
- authUrl = mkOption { - type = types.str; - default = "http://${cfg.hostname}:${toString cfg.port}"; - description = "Hostname or IP address for the SourceBot web server to bind to"; - }; + authUrl = mkOption { + type = types.str; + default = "http://localhost:${toString cfg.port}"; + description = "Base URL for authentication callbacks"; + };
163-163: Import makeBinPath function.The
makeBinPathfunction is used but not imported.- "PATH=${makeBinPath (with pkgs; [prisma openssl])}" + "PATH=${lib.makeBinPath (with pkgs; [prisma openssl])}"
196-199: Avoid hardcoded secrets in environment.Using hardcoded placeholder secrets even as defaults is a security risk. Consider requiring users to provide an envFile or generate random secrets.
Consider either:
- Making
envFilerequired when auth is enabled- Generating random secrets at runtime
- Providing clear documentation that these are insecure defaults
Would you like me to implement a solution that generates secure random secrets?
Also applies to: 230-233
220-220: Use consistent databaseUrl and authUrl references.Both services should use the configured
cfg.databaseUrlandcfg.authUrlvalues instead of manually constructing URLs, creating inconsistency.- "DATABASE_URL=postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot" + "DATABASE_URL=${cfg.databaseUrl}" - "AUTH_URL=http://${cfg.hostname}:${toString cfg.port}" + "AUTH_URL=${cfg.authUrl}"Also applies to: 228-228
🧹 Nitpick comments (1)
nix/nixosModule.nix (1)
52-52: Fix typo in description.There's a typo in "Firwall" which should be "Firewall".
- description = "Open Firwall ports for SourceBot"; + description = "Open Firewall ports for SourceBot";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
flake.nix(1 hunks)nix/nixosModule.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- flake.nix
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
nix/nixosModule.nix
🧠 Learnings (1)
nix/nixosModule.nix (1)
Learnt from: drew-u410
PR: #328
File: CONTRIBUTING.md:5-5
Timestamp: 2025-06-03T00:22:00.498Z
Learning: In the sourcebot repository CONTRIBUTING.md file, the maintainer prefers using "NodeJS" spelling instead of the official "Node.js" branding, and prefers not to add alt text to favicon images in the installation instructions.
🧬 Code Graph Analysis (1)
nix/nixosModule.nix (1)
packages/schemas/src/v3/index.type.ts (1)
SourcebotConfig(22-41)
| }; | ||
| databaseUrl = mkOption { | ||
| type = types.nullOr types.str; | ||
| default = "postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot"; |
There was a problem hiding this comment.
Fix string interpolation in default value.
The default value uses string interpolation syntax that won't work properly in Nix attribute definitions.
- default = "postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot";
+ default = null;Then add logic in the config section to construct the URL dynamically:
config = mkIf cfg.enable {
# ... existing config ...
# Set database URL if not provided
services.sourcebot.databaseUrl = mkDefault "postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot";🤖 Prompt for AI Agents
In nix/nixosModule.nix at line 81, the default database URL uses incorrect
string interpolation syntax for Nix. Replace the current interpolation with
proper Nix string concatenation using the + operator and toString function to
dynamically construct the URL. Additionally, add logic in the config section to
set services.sourcebot.databaseUrl with mkDefault and build the URL dynamically
based on config.services.postgresql.settings.port when cfg.enable is true.
| "AUTH_CREDENTIALS_LOGIN_ENABLED=${boolToString cfg.authEnabled}" | ||
| "SOURCEBOT_TELEMETRY_DISABLED=${boolToString cfg.telemetryDisabled}" | ||
| "SOURCEBOT_PUBLIC_KEY_PATH=${cfg.package}/public.pem" | ||
| "AUTH_URL=http://${cfg.hostname}:${toString cfg.port}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent AUTH_URL configuration.
Both web and backend services should use the same AUTH_URL configuration. Currently, the web service uses it correctly but the backend service duplicates the manual construction.
In the web service, change:
- "AUTH_URL=http://${cfg.hostname}:${toString cfg.port}"
+ "AUTH_URL=${cfg.authUrl}"The backend service already needs this fix as noted in the duplicate comment above.
Also applies to: 227-227
🤖 Prompt for AI Agents
In nix/nixosModule.nix at lines 194 and 227, the AUTH_URL is manually
constructed inconsistently between web and backend services. Refactor both
services to use a single, consistent AUTH_URL configuration by referencing the
same variable or expression instead of duplicating the manual string
construction. This ensures uniformity and easier maintenance.
abraverm
force-pushed
the
flake.nix
branch
3 times, most recently
from
52c0154 to
3b20128
Compare
abraverm
force-pushed
the
flake.nix
branch
3 times, most recently
from
8408caa to
568395a
Compare
|
Hey @abraverm out of curiosity are you still using this for your deployment? Would be great to have this merged if so |
|
Yes, and I will address the comments in the comming week. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/docs/deployment-guide.mdx`:
- Around line 176-178: The example sets extremely high concurrency values for
maxRepoIndexingJobConcurrency, maxConnectionSyncJobConcurrency, and
maxRepoGarbageCollectionJobConcurrency (1000) which contradicts the “8 default”
comment; change these example values to reasonable numbers (e.g., 8 or 16) or
revert to the documented defaults and update the inline comments to match the
chosen values so users won’t copy unsafe settings; locate and edit the three
symbols maxRepoIndexingJobConcurrency, maxConnectionSyncJobConcurrency, and
maxRepoGarbageCollectionJobConcurrency in the example block and adjust both the
numeric values and their trailing comments to be consistent.
- Around line 185-186: Update the "Complete onboarding" Step to add a
one-sentence clarification about port differences: explain that the NixOS module
intentionally defaults to port 7734 (hence the documentation uses
http://localhost:7734) while the container deployment exposes port 3000, and
mention that this is configured in the NixOS module so users should not expect
the same port as containers. Reference the Step title "Complete onboarding" when
making the edit and keep the note brief and factual.
In `@nix/nixosModule.nix`:
- Around line 218-220: Replace the hardcoded PostgreSQL connection string used
for the backend service env var with the configured value `cfg.databaseUrl`
(same approach as the web service). Locate the
`"DATABASE_URL=postgresql://sourcebot@localhost:${toString
config.services.postgresql.settings.port}/sourcebot"` entry and change it to
reference `cfg.databaseUrl` (interpolated into the env list) so user overrides
of `databaseUrl` are honored; ensure any necessary string interpolation matches
how `cfg.databaseUrl` is used elsewhere in this module.
🧹 Nitpick comments (4)
nix/sourcebot.nix (1)
93-96: Remove or address commented-out package copies.These commented lines for
crypto,error, andloggerpackages should either be removed if no longer needed, or uncommented if required. Leaving them creates confusion about the intended package structure.flake.nix (1)
15-15: Consider documenting x86_64-linux-only limitation.The flake only supports
x86_64-linux. If aarch64-linux (ARM) support is intended in the future, this would need to be expanded. For now, this is acceptable but should be documented.docs/docs/deployment-guide.mdx (2)
5-5: Unused import.
SupportedPlatformsis imported but never used in this document. Either use it or remove the import.Proposed fix
--- title: "Deployment guide" --- - -import SupportedPlatforms from '/snippets/platform-support.mdx'
131-137: Indentation inconsistency in agenix example.The text description and code block for the environment file format have inconsistent indentation compared to the sops-nix example above. This may cause rendering issues in the documentation.
Proposed fix
- `sourcebot.age` file should be an environment file in the format: + `sourcebot.age` file should be an environment file in the format: - ```bash wrap icon="terminal" Environment file format + ```bash wrap icon="terminal" Environment file format
| maxRepoIndexingJobConcurrency = 1000; # 8 default | ||
| maxConnectionSyncJobConcurrency = 1000; # 8 default | ||
| maxRepoGarbageCollectionJobConcurrency = 1000; # 8 default |
There was a problem hiding this comment.
Unusually high concurrency values in example.
The concurrency settings are set to 1000 while comments indicate the default is 8. These values seem extremely high for an example configuration and could cause resource exhaustion if users copy this verbatim. Consider using more reasonable values or the defaults.
Proposed fix with more reasonable example values
settings = {
resyncConnectionIntervalMs = 1000 * 60 * 60 * 24 * 7; # 1 week
reindexIntervalMs = 1000 * 60 * 60 * 24 * 7; # 1 week
- maxRepoIndexingJobConcurrency = 1000; # 8 default
- maxConnectionSyncJobConcurrency = 1000; # 8 default
- maxRepoGarbageCollectionJobConcurrency = 1000; # 8 default
+ maxRepoIndexingJobConcurrency = 8;
+ maxConnectionSyncJobConcurrency = 8;
+ maxRepoGarbageCollectionJobConcurrency = 8;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| maxRepoIndexingJobConcurrency = 1000; # 8 default | |
| maxConnectionSyncJobConcurrency = 1000; # 8 default | |
| maxRepoGarbageCollectionJobConcurrency = 1000; # 8 default | |
| maxRepoIndexingJobConcurrency = 8; | |
| maxConnectionSyncJobConcurrency = 8; | |
| maxRepoGarbageCollectionJobConcurrency = 8; |
🤖 Prompt for AI Agents
In `@docs/docs/deployment-guide.mdx` around lines 176 - 178, The example sets
extremely high concurrency values for maxRepoIndexingJobConcurrency,
maxConnectionSyncJobConcurrency, and maxRepoGarbageCollectionJobConcurrency
(1000) which contradicts the “8 default” comment; change these example values to
reasonable numbers (e.g., 8 or 16) or revert to the documented defaults and
update the inline comments to match the chosen values so users won’t copy unsafe
settings; locate and edit the three symbols maxRepoIndexingJobConcurrency,
maxConnectionSyncJobConcurrency, and maxRepoGarbageCollectionJobConcurrency in
the example block and adjust both the numeric values and their trailing comments
to be consistent.
| <Step title="Complete onboarding"> | ||
| Navigate to `http://localhost:7734` and complete the onboarding flow. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the deployment guide file
fd "deployment-guide" --type f
# Then read the relevant lines to understand the context
echo "=== Checking container deployment port (around line 69) ==="
sed -n '60,80p' docs/docs/deployment-guide.mdx
echo -e "\n=== Checking NixOS deployment port (around line 185-186) ==="
sed -n '175,195p' docs/docs/deployment-guide.mdxRepository: sourcebot-dev/sourcebot
Length of output: 2014
🏁 Script executed:
#!/bin/bash
# Search for NixOS module files and port configuration
echo "=== Searching for NixOS module files ==="
fd "\.nix$" | grep -E "(module|nixos|deployment)" | head -20
echo -e "\n=== Searching for port 7734 in the codebase ==="
rg -n "7734" --type-list
echo -e "\n=== Searching for port 3000 in the codebase ==="
rg -n "3000" --type-listRepository: sourcebot-dev/sourcebot
Length of output: 1040
🏁 Script executed:
#!/bin/bash
# Check the NixOS module for port configuration
echo "=== Reading NixOS module file ==="
cat -n nix/nixosModule.nix | head -100
echo -e "\n=== Searching for port configuration in nixos module ==="
grep -n "port\|7734\|3000" nix/nixosModule.nix || echo "No matches found"
echo -e "\n=== Check full nixos module ==="
wc -l nix/nixosModule.nixRepository: sourcebot-dev/sourcebot
Length of output: 4957
🏁 Script executed:
#!/bin/bash
# Correct search for port references
echo "=== Searching for port 7734 ==="
rg "7734"
echo -e "\n=== Searching for port 3000 ==="
rg "3000"
echo -e "\n=== Searching for port configuration in general ==="
rg "port.*=" --type nix | head -20Repository: sourcebot-dev/sourcebot
Length of output: 29636
Consider clarifying why NixOS deployment uses port 7734 instead of 3000.
The NixOS deployment defaults to port 7734, while the container deployment uses port 3000. Since the NixOS module explicitly configures 7734 as the default port, this is intentional. However, the documentation could benefit from a brief note explaining this difference to avoid user confusion when comparing deployment options.
🤖 Prompt for AI Agents
In `@docs/docs/deployment-guide.mdx` around lines 185 - 186, Update the "Complete
onboarding" Step to add a one-sentence clarification about port differences:
explain that the NixOS module intentionally defaults to port 7734 (hence the
documentation uses http://localhost:7734) while the container deployment exposes
port 3000, and mention that this is configured in the NixOS module so users
should not expect the same port as containers. Reference the Step title
"Complete onboarding" when making the edit and keep the note brief and factual.
| "DATA_DIR=${cfg.dataDir}" | ||
| "DATA_CACHE_DIR=${cfg.dataCacheDir}" | ||
| "DATABASE_URL=postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot" |
There was a problem hiding this comment.
Backend service uses hardcoded DATABASE_URL instead of cfg.databaseUrl.
The web service correctly uses cfg.databaseUrl at line 186, but the backend service hardcodes the PostgreSQL URL. This creates inconsistency if users override databaseUrl.
"DATA_DIR=${cfg.dataDir}"
"DATA_CACHE_DIR=${cfg.dataCacheDir}"
- "DATABASE_URL=postgresql://sourcebot@localhost:${toString config.services.postgresql.settings.port}/sourcebot"
+ "DATABASE_URL=${cfg.databaseUrl}"🤖 Prompt for AI Agents
In `@nix/nixosModule.nix` around lines 218 - 220, Replace the hardcoded PostgreSQL
connection string used for the backend service env var with the configured value
`cfg.databaseUrl` (same approach as the web service). Locate the
`"DATABASE_URL=postgresql://sourcebot@localhost:${toString
config.services.postgresql.settings.port}/sourcebot"` entry and change it to
reference `cfg.databaseUrl` (interpolated into the env list) so user overrides
of `databaseUrl` are honored; ensure any necessary string interpolation matches
how `cfg.databaseUrl` is used elsewhere in this module.
|
@msukkari please take a look at the latest change. If you merge this, you would need to keep the hash up to date everytime you update the dependencies. I can add a script to make it simple for the developers, or add as a Github test that would block a commit if a package can't be build and print the needed changes (new hash and dependency file). |
| missingHashes = ./missing-hashes.json; | ||
| offlineCache = pkgs.yarn-berry.fetchYarnBerryDeps { | ||
| inherit (finalAttrs) src missingHashes; | ||
| hash = "sha256-FpSmCMvHRDea+UOdVIp/c0ydF2zSWjbcU056oBrP0/Y="; |
There was a problem hiding this comment.
This needs to be updated in the future
There was a problem hiding this comment.
This file needs to be generated in future changes
2 participants
Package Sourcebot with Nix, NixOS module for deployment, integration test and microvm.
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.