feat(nix): add Nix flake for reproducible builds and containers#727
Open
randomizedcoder wants to merge 15 commits intoNVIDIA:mainfrom
Open
feat(nix): add Nix flake for reproducible builds and containers#727randomizedcoder wants to merge 15 commits intoNVIDIA:mainfrom
randomizedcoder wants to merge 15 commits intoNVIDIA:mainfrom
Conversation
Add modular Nix infrastructure that provides: - Dev shell with all build/lint/test tools (nix develop) - NemoClaw package build mirroring Dockerfile's 2-stage pattern - OCI container image via dockerTools.buildLayeredImage - 27-check container smoke test suite - Sphinx documentation build (best-effort) OpenClaw is sourced from nixpkgs (pkgs.openclaw). All config is centralized in constants.nix with filtered sources for cache efficiency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
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 pull request introduces a comprehensive Nix flake infrastructure for NemoClaw, establishing a modular build system that defines package compilation, OCI container image generation, development environments, and automated testing/scanning capabilities. It adds a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
nix/README.md (2)
169-177: Add language specifier to fenced code block.Same issue with the module dependencies diagram.
Proposed fix
-``` +```text constants.nix --+---> source-filter.nix --+---> package.nix --+---> container.nix🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/README.md` around lines 169 - 177, Update the fenced code block(s) showing the module dependency diagram to include a language specifier (e.g., change ``` to ```text) so the diagram renders as plain text; locate the diagram containing "constants.nix --+---> source-filter.nix --+---> package.nix --+---> container.nix" and replace its opening fence with a language-tagged fence (e.g., ```text) for proper formatting.
153-163: Add language specifier to fenced code block.The architecture diagram code block should have a language specifier for consistency with markdown linting rules. Since this is ASCII art, use
textorplaintext.Proposed fix
-``` +```text flake.nix # Coordinator — imports from ./nix/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/README.md` around lines 153 - 163, The fenced ASCII-art code block in README.md lacks a language specifier; update the block fence that contains the architecture diagram so it starts with ```text (or ```plaintext) instead of just ```, ensuring markdown linters recognize it as plain text and the diagram remains unchanged; locate the diagram block in nix/README.md and add the language token to the opening fence.flake.nix (2)
62-65: Consider adding container-test to checks.The
checksattribute only validatesnemoclawandshell. Consider addingcontainer-testto ensure container builds are validated duringnix flake check, though this would require Docker to be available in the CI environment.Proposed addition (if Docker is available in CI)
checks = { inherit nemoclaw; shell = self.devShells.${system}.default; + # Uncomment if Docker is available in CI + # inherit container-test; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 62 - 65, Add the missing container-test to the flake's checks attribute so container builds are validated by `nix flake check`; update the `checks` block that currently only inherits `nemoclaw` and `shell` (the `shell` entry is `self.devShells.${system}.default`) to also include `container-test`, and if CI cannot provide Docker ensure you gate this addition behind a CI condition or document that Docker must be available before enabling `container-test`.
14-16: Document why openclaw is marked insecure.Adding
permittedInsecurePackagesworks but it's good practice to add a comment explaining why this package is marked insecure in nixpkgs and whether the security concern applies to this use case.Proposed documentation
pkgs = import nixpkgs { inherit system; + # openclaw-2026.3.12 is marked insecure in nixpkgs due to [reason]. + # This is acceptable for NemoClaw because [justification]. config.permittedInsecurePackages = [ "openclaw-2026.3.12" ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 14 - 16, Add a comment explaining why openclaw-2026.3.12 is listed in config.permittedInsecurePackages: state the specific CVE or security advisory (or the root cause such as outdated dependency/licensing/build issue) from nixpkgs, whether the vulnerability affects the features we use, and any mitigation or rationale for accepting the risk (e.g., offline usage, sandboxing, pinned version awaiting upstream fix). Place this comment immediately above the config.permittedInsecurePackages block referencing the package name openclaw-2026.3.12 and include a link or reference to the upstream issue/PR if available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nix/container.nix`:
- Around line 107-109: The chmod 1777 on .${constants.paths.openclawConfig}
contradicts the intent to prevent sandbox writes because it makes the directory
world-writable; change the permissions to a root-only writable/readable mode
(e.g. 0755) and remove the sticky bit so the sandbox user cannot create or
modify files there. In the block that calls chown -R root:root
.${constants.paths.openclawConfig} and chmod 1777
.${constants.paths.openclawConfig}, replace the chmod with a mode that enforces
read/execute for others (0755) and ensure all files inside are owned by root and
not group/other-writable. Ensure any subsequent code that expects
world-writeability is adjusted accordingly.
---
Nitpick comments:
In `@flake.nix`:
- Around line 62-65: Add the missing container-test to the flake's checks
attribute so container builds are validated by `nix flake check`; update the
`checks` block that currently only inherits `nemoclaw` and `shell` (the `shell`
entry is `self.devShells.${system}.default`) to also include `container-test`,
and if CI cannot provide Docker ensure you gate this addition behind a CI
condition or document that Docker must be available before enabling
`container-test`.
- Around line 14-16: Add a comment explaining why openclaw-2026.3.12 is listed
in config.permittedInsecurePackages: state the specific CVE or security advisory
(or the root cause such as outdated dependency/licensing/build issue) from
nixpkgs, whether the vulnerability affects the features we use, and any
mitigation or rationale for accepting the risk (e.g., offline usage, sandboxing,
pinned version awaiting upstream fix). Place this comment immediately above the
config.permittedInsecurePackages block referencing the package name
openclaw-2026.3.12 and include a link or reference to the upstream issue/PR if
available.
In `@nix/README.md`:
- Around line 169-177: Update the fenced code block(s) showing the module
dependency diagram to include a language specifier (e.g., change ``` to ```text)
so the diagram renders as plain text; locate the diagram containing
"constants.nix --+---> source-filter.nix --+---> package.nix --+--->
container.nix" and replace its opening fence with a language-tagged fence (e.g.,
```text) for proper formatting.
- Around line 153-163: The fenced ASCII-art code block in README.md lacks a
language specifier; update the block fence that contains the architecture
diagram so it starts with ```text (or ```plaintext) instead of just ```,
ensuring markdown linters recognize it as plain text and the diagram remains
unchanged; locate the diagram block in nix/README.md and add the language token
to the opening fence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d5237ad-e44f-4874-b99f-363fe5af9663
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.gitignoreflake.nixnix/README.mdnix/constants.nixnix/container-test.nixnix/container.nixnix/docs.nixnix/package.nixnix/shell.nixnix/source-filter.nix
- Remove flake.nix and flake.lock from excludePatterns so changes to locked inputs and build logic properly invalidate the source hash - Fix chmod 1777 (world-writable) on .openclaw to 755/644 so the sandbox user gets read+execute only, matching the security intent - Add text language specifier to bare code fences in README - Document why openclaw is marked as insecure in permittedInsecurePackages - Add commented container-test entry in flake checks (requires Docker) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
Addressing CodeRabbit Review FeedbackPushed commit [Actionable] Fix
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nix/container.nix (1)
119-121: Consider whether interactive shell entrypoint is intentional.
Entrypoint = [ "/bin/bash" ]with emptyCmdmeans the container starts an interactive shell by default. The generatednemoclaw-startscript is included at/usr/local/bin/nemoclaw-startbut not automatically invoked.If this is for development flexibility, this is fine. For production use, consider:
Option: Use start script as entrypoint with bash fallback
config = { - Entrypoint = [ "/bin/bash" ]; - Cmd = [ ]; + Entrypoint = [ "/usr/local/bin/nemoclaw-start" ]; + Cmd = [ ]; # Override with /bin/bash for shell access User = "${toString constants.user.uid}:${toString constants.user.gid}";Users can still get a shell with
docker run --entrypoint /bin/bash ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/container.nix` around lines 119 - 121, The container currently sets Entrypoint = [ "/bin/bash" ] and an empty Cmd which yields an interactive shell by default; change the Entrypoint to use the included start script (Entrypoint -> "/usr/local/bin/nemoclaw-start") so the container runs your app by default, and set Cmd to a sensible fallback (e.g., "/bin/bash") so a shell is still available when no args are provided; update the Entrypoint/Cmd entries in the Nix expression where Entrypoint and Cmd are defined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@nix/container.nix`:
- Around line 119-121: The container currently sets Entrypoint = [ "/bin/bash" ]
and an empty Cmd which yields an interactive shell by default; change the
Entrypoint to use the included start script (Entrypoint ->
"/usr/local/bin/nemoclaw-start") so the container runs your app by default, and
set Cmd to a sensible fallback (e.g., "/bin/bash") so a shell is still available
when no args are provided; update the Entrypoint/Cmd entries in the Nix
expression where Entrypoint and Cmd are defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a063b225-f543-4ddd-8042-0a030fd335f8
📒 Files selected for processing (4)
flake.nixnix/README.mdnix/constants.nixnix/container.nix
✅ Files skipped from review due to trivial changes (3)
- nix/README.md
- flake.nix
- nix/constants.nix
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
nix/container-test.nix (1)
7-10: Declaregrepin the wrapper closure.
check_output()shells out togrepon Line 53, but the app closure only provisions Docker/coreutils/gawk. Please addgnugrephere sonix run .#container-testdoes not depend on the caller's ambientPATH.♻️ Minimal diff
{ writeShellApplication, docker, coreutils, gawk, + gnugrep, constants, container, }: runtimeInputs = [ docker coreutils gawk + gnugrep ];Also applies to: 17-21, 49-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/container-test.nix` around lines 7 - 10, The wrapper closure that builds the container test currently lists docker, coreutils, gawk, and constants but omits gnugrep, so check_output() (which shells out to grep) can accidentally rely on the caller's PATH; add gnugrep to the closure's argument list and to the set of inputs used when creating the wrapper so the script invokes gnugrep (or provides a symlinked grep) from the Nix-provided runtime rather than the ambient environment—update the same omission wherever the closure is declared (the other occurrences noted around the ranges that include the wrapper and check_output() references).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nix/container-test.nix`:
- Around line 79-82: The script currently creates the container with a fixed
name via the docker create call (see the CONTAINER variable and the line using
docker create --name nemoclaw-nix-test "$IMAGE" -c "sleep 300"); remove the
--name nemoclaw-nix-test flag so docker returns an anonymous container ID into
CONTAINER and subsequent docker start "$CONTAINER" and cleanup logic can operate
on that ID without name collisions in reruns or parallel jobs.
In `@nix/container.nix`:
- Around line 131-140: The current sequence of chown/chmod leaves the user's
home (${toString constants.user.home}) writable so the sandbox user can
delete/replace the root-owned ${toString constants.paths.openclawConfig}
directory; also ${toString constants.paths.openclawConfig} is made read-only but
application runtime needs to write openclaw.json (see "write `openclaw.json` on
first run"). Fix by making the parent directory of ${toString
constants.paths.openclawConfig} non-writable by the sandbox user (remove or
change the chown on ${toString constants.user.home} for that parent) and move
any runtime-writable files (e.g., openclaw.json) into ${toString
constants.paths.openclawData} with symlinks from ${toString
constants.paths.openclawConfig} to the data files; update the chown/chmod steps
so ${toString constants.paths.openclawConfig} is root:root + 755/644 while
${toString constants.paths.openclawData} remains owned by ${toString
constants.user.uid}:${toString constants.user.gid} and writable.
---
Nitpick comments:
In `@nix/container-test.nix`:
- Around line 7-10: The wrapper closure that builds the container test currently
lists docker, coreutils, gawk, and constants but omits gnugrep, so
check_output() (which shells out to grep) can accidentally rely on the caller's
PATH; add gnugrep to the closure's argument list and to the set of inputs used
when creating the wrapper so the script invokes gnugrep (or provides a symlinked
grep) from the Nix-provided runtime rather than the ambient environment—update
the same omission wherever the closure is declared (the other occurrences noted
around the ranges that include the wrapper and check_output() references).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7fae7d5-7a6a-42e2-aec6-a66220875a4b
📒 Files selected for processing (8)
flake.nixnix/constants.nixnix/container-test.nixnix/container.nixnix/docs.nixnix/package.nixnix/shell.nixnix/source-filter.nix
✅ Files skipped from review due to trivial changes (4)
- nix/source-filter.nix
- nix/docs.nix
- flake.nix
- nix/constants.nix
🚧 Files skipped from review as they are similar to previous changes (2)
- nix/shell.nix
- nix/package.nix
Comment on lines
+79
to
+82
| # ── Start container ───────────────────────────────────────── | ||
| echo "Starting container..." | ||
| CONTAINER=$(docker create --name nemoclaw-nix-test "$IMAGE" -c "sleep 300") | ||
| docker start "$CONTAINER" |
Contributor
There was a problem hiding this comment.
Drop the fixed container name.
Using --name nemoclaw-nix-test makes reruns and parallel jobs fail whenever a stale container with that name already exists. The returned container ID is already enough for cleanup.
🔧 Minimal diff
- CONTAINER=$(docker create --name nemoclaw-nix-test "$IMAGE" -c "sleep 300")
+ CONTAINER=$(docker create "$IMAGE" -c "sleep 300")📝 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.
Suggested change
| # ── Start container ───────────────────────────────────────── | |
| echo "Starting container..." | |
| CONTAINER=$(docker create --name nemoclaw-nix-test "$IMAGE" -c "sleep 300") | |
| docker start "$CONTAINER" | |
| # ── Start container ───────────────────────────────────────── | |
| echo "Starting container..." | |
| CONTAINER=$(docker create "$IMAGE" -c "sleep 300") | |
| docker start "$CONTAINER" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nix/container-test.nix` around lines 79 - 82, The script currently creates
the container with a fixed name via the docker create call (see the CONTAINER
variable and the line using docker create --name nemoclaw-nix-test "$IMAGE" -c
"sleep 300"); remove the --name nemoclaw-nix-test flag so docker returns an
anonymous container ID into CONTAINER and subsequent docker start "$CONTAINER"
and cleanup logic can operate on that ID without name collisions in reruns or
parallel jobs.
Comment on lines
+131
to
+140
| # Sandbox user owns their home | ||
| chown -R ${toString constants.user.uid}:${toString constants.user.gid} .${constants.user.home} | ||
|
|
||
| # DAC lockdown: root owns .openclaw so sandbox user cannot modify config | ||
| chown -R root:root .${constants.paths.openclawConfig} | ||
| find .${constants.paths.openclawConfig} -type d -exec chmod 755 {} + | ||
| find .${constants.paths.openclawConfig} -type f -exec chmod 644 {} + | ||
|
|
||
| # .openclaw-data stays writable by sandbox user | ||
| chown -R ${toString constants.user.uid}:${toString constants.user.gid} .${constants.paths.openclawData} |
Contributor
There was a problem hiding this comment.
This ownership model does not actually lock down .openclaw.
Because Line 132 leaves ${constants.user.home} writable by the sandbox user, they can still rename/delete the root-owned .openclaw entry from its parent directory and replace it. At the same time, Lines 135-137 make .openclaw read-only, which conflicts with Line 44's "write openclaw.json on first run" behavior. Please move runtime-writable files like openclaw.json into .openclaw-data behind symlinks, and keep the parent of .openclaw non-writable if you want the lockdown to hold.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nix/container.nix` around lines 131 - 140, The current sequence of
chown/chmod leaves the user's home (${toString constants.user.home}) writable so
the sandbox user can delete/replace the root-owned ${toString
constants.paths.openclawConfig} directory; also ${toString
constants.paths.openclawConfig} is made read-only but application runtime needs
to write openclaw.json (see "write `openclaw.json` on first run"). Fix by making
the parent directory of ${toString constants.paths.openclawConfig} non-writable
by the sandbox user (remove or change the chown on ${toString
constants.user.home} for that parent) and move any runtime-writable files (e.g.,
openclaw.json) into ${toString constants.paths.openclawData} with symlinks from
${toString constants.paths.openclawConfig} to the data files; update the
chown/chmod steps so ${toString constants.paths.openclawConfig} is root:root +
755/644 while ${toString constants.paths.openclawData} remains owned by
${toString constants.user.uid}:${toString constants.user.gid} and writable.
wscurran
added
Docker
Contributor
|
Thanks for the proposed fix, it's great to see the addition of Nix flake infrastructure, and the changes are well-documented and easy to follow, it's a good example of how to improve reproducibility and containerization. |
Author
|
Also nix for OpenShell, but it has been auto closed |
…Y_PORT The OpenShell gateway port was hardcoded to 8080 in both the preflight port check and the gateway start command. Users whose port 8080 is already occupied had no way to proceed with onboarding. Read OPENSHELL_GATEWAY_PORT from the environment (default 8080) and pass it through to `openshell gateway start --port`. This aligns with the upstream OpenShell CLI which already supports --port. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The OCI container previously used /bin/bash as its entrypoint, requiring users to manually invoke nemoclaw-start. Change the entrypoint to /usr/local/bin/nemoclaw-start so the container runs the app by default. Users can still get a shell with: docker run --entrypoint /bin/bash ... Also run the gateway in the foreground (exec) so the container stays alive with the gateway as PID 1, and update the container smoke test to override the entrypoint for its sleep-based test harness and verify the new entrypoint config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The nix-built nemoclaw package was missing files that onboard.js needs to build the sandbox Docker image: the Dockerfile, TypeScript source files (tsconfig.json, src/, package-lock.json). Also fix two nix-store compatibility issues: - Patch cp -r calls to use --no-preserve=mode so read-only nix store files become writable when copied into the temp build context - Restore portable shebangs (#!/usr/bin/env bash) in postFixup for scripts that get copied into non-nix Docker containers Wire OPENSHELL_GATEWAY_PORT through the makeWrapper with a default from constants.nix (8080). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a quick-start comment block to flake.nix with common nix commands. Update nix/README.md to reflect the new nemoclaw-start entrypoint, add a "Running with Nix" section documenting the .# -- syntax for passing subcommands, and mention the entrypoint check in the smoke test description. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Added a fix for the hard coded 8080 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
nix/container-test.nix (1)
79-82:⚠️ Potential issue | 🟠 MajorDrop the fixed container name to avoid rerun/parallel collisions.
Line 81 still hard-codes
--name nemoclaw-nix-test, which can fail when a stale container exists or in concurrent jobs.🔧 Minimal fix
- CONTAINER=$(docker create --name nemoclaw-nix-test --entrypoint /bin/bash "$IMAGE" -c "sleep 300") + CONTAINER=$(docker create --entrypoint /bin/bash "$IMAGE" -c "sleep 300")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/container-test.nix` around lines 79 - 82, The script currently hard-codes the container name via --name nemoclaw-nix-test which causes collisions; change it to use a unique name or omit --name and rely on the container ID. Generate a unique identifier (e.g., with mktemp/uuid/date+$$) into a CONTAINER_NAME variable and pass --name "$CONTAINER_NAME" to the docker create call (or drop --name and use the returned CONTAINER value), then use that variable in the docker start invocation and any cleanup logic so reruns and parallel jobs do not conflict with a stale fixed name.nix/container.nix (1)
131-140:⚠️ Potential issue | 🔴 Critical
.openclawlockdown is still bypassable via writable parent directory.Line 132 makes
${constants.user.home}writable by the sandbox user; they can rename/replace the root-owned.openclawentry despite Lines 135-137. This breaks the stated DAC lockdown intent.🔧 Lockdown-oriented adjustment
- # Sandbox user owns their home - chown -R ${toString constants.user.uid}:${toString constants.user.gid} .${constants.user.home} + # Keep home parent immutable so .openclaw cannot be replaced + chown root:root .${constants.user.home} + chmod 755 .${constants.user.home} # DAC lockdown: root owns .openclaw so sandbox user cannot modify config chown -R root:root .${constants.paths.openclawConfig} find .${constants.paths.openclawConfig} -type d -exec chmod 755 {} + find .${constants.paths.openclawConfig} -type f -exec chmod 644 {} + # .openclaw-data stays writable by sandbox user chown -R ${toString constants.user.uid}:${toString constants.user.gid} .${constants.paths.openclawData} + chmod -R u+rwX,go-rw .${constants.paths.openclawData}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/container.nix` around lines 131 - 140, The DAC lockdown is bypassable because the sandbox user owns ${toString constants.user.home} and can rename/replace root-owned .${constants.paths.openclawConfig}; change ownership of the home directory to root and then explicitly restore user ownership only for the intended writable subpaths (e.g., .${constants.paths.openclawData}) so the user cannot modify or replace .${constants.paths.openclawConfig}; update the chown/chmod sequence around ${toString constants.user.home}, .${constants.paths.openclawConfig}, and .${constants.paths.openclawData} to first chown the home to root:root (and set restrictive perms), then chown only .${constants.paths.openclawData} back to ${toString constants.user.uid}:${toString constants.user.gid} and ensure .${constants.paths.openclawConfig} remains root:root with 755 for dirs and 644 for files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nix/container-test.nix`:
- Line 97: The version check uses a Node one-liner with an undefined global
'assert' and will crash; update the command supplied to check/run_in so it
doesn't rely on assert but instead exits with status 0 when process.version
starts with the expected string and non‑zero otherwise (use
process.version.startsWith('v' + constants.nodeVersion + '.') and call
process.exit accordingly). Replace the existing node -e string in the check
invocation that references constants.nodeVersion to perform the explicit
exit-code check rather than using assert.
---
Duplicate comments:
In `@nix/container-test.nix`:
- Around line 79-82: The script currently hard-codes the container name via
--name nemoclaw-nix-test which causes collisions; change it to use a unique name
or omit --name and rely on the container ID. Generate a unique identifier (e.g.,
with mktemp/uuid/date+$$) into a CONTAINER_NAME variable and pass --name
"$CONTAINER_NAME" to the docker create call (or drop --name and use the returned
CONTAINER value), then use that variable in the docker start invocation and any
cleanup logic so reruns and parallel jobs do not conflict with a stale fixed
name.
In `@nix/container.nix`:
- Around line 131-140: The DAC lockdown is bypassable because the sandbox user
owns ${toString constants.user.home} and can rename/replace root-owned
.${constants.paths.openclawConfig}; change ownership of the home directory to
root and then explicitly restore user ownership only for the intended writable
subpaths (e.g., .${constants.paths.openclawData}) so the user cannot modify or
replace .${constants.paths.openclawConfig}; update the chown/chmod sequence
around ${toString constants.user.home}, .${constants.paths.openclawConfig}, and
.${constants.paths.openclawData} to first chown the home to root:root (and set
restrictive perms), then chown only .${constants.paths.openclawData} back to
${toString constants.user.uid}:${toString constants.user.gid} and ensure
.${constants.paths.openclawConfig} remains root:root with 755 for dirs and 644
for files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f16a1f1-5dd5-4ba2-adb2-00fe5ccaf7bb
📒 Files selected for processing (8)
bin/lib/onboard.jsflake.nixnix/README.mdnix/constants.nixnix/container-test.nixnix/container.nixnix/package.nixscripts/nemoclaw-start.sh
✅ Files skipped from review due to trivial changes (3)
- bin/lib/onboard.js
- nix/README.md
- nix/constants.nix
🚧 Files skipped from review as they are similar to previous changes (1)
- nix/package.nix
| check "bash is on PATH" run_in "command -v bash" | ||
|
|
||
| # Runtime versions | ||
| check "node is v${constants.nodeVersion}.x" run_in "node -e \"assert(process.version.startsWith('v${constants.nodeVersion}.'))\"" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js v22, is assertavailable as a global innode -escripts, or must it be imported fromnode:assert?
💡 Result:
In Node.js v22, assert is not a global in normal scripts (including node -e ...), so you must import/require it (e.g. const assert = require('node:assert') or import { strict as assert } from 'node:assert'). [1][2]
The Node REPL is different: it can auto-load core modules on first use (e.g. typing fs loads node:fs), but that behavior is REPL-specific and doesn’t make assert a global for node -e scripts. [3]
Example (node -e):
node -e "const assert = require('node:assert'); assert.ok(true)"Sources: [1] [2] [3]
🏁 Script executed:
# Find and read the nix file to understand the context
find . -name "container-test.nix" -type fRepository: NVIDIA/NemoClaw
Length of output: 83
🏁 Script executed:
# Look for constants.nodeVersion definition
rg "nodeVersion" --type nix -B 2 -A 2Repository: NVIDIA/NemoClaw
Length of output: 650
Node version check will fail in Node.js v22 — assert is not globally available in node -e scripts and must be explicitly imported. The current code will throw a ReferenceError: assert is not defined.
Use an explicit exit code check instead:
Fix
- check "node is v${constants.nodeVersion}.x" run_in "node -e \"assert(process.version.startsWith('v${constants.nodeVersion}.'))\""
+ check "node is v${constants.nodeVersion}.x" run_in "node -e \"process.exit(process.version.startsWith('v${constants.nodeVersion}.') ? 0 : 1)\""📝 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.
Suggested change
| check "node is v${constants.nodeVersion}.x" run_in "node -e \"assert(process.version.startsWith('v${constants.nodeVersion}.'))\"" | |
| check "node is v${constants.nodeVersion}.x" run_in "node -e \"process.exit(process.version.startsWith('v${constants.nodeVersion}.') ? 0 : 1)\"" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nix/container-test.nix` at line 97, The version check uses a Node one-liner
with an undefined global 'assert' and will crash; update the command supplied to
check/run_in so it doesn't rely on assert but instead exits with status 0 when
process.version starts with the expected string and non‑zero otherwise (use
process.version.startsWith('v' + constants.nodeVersion + '.') and call
process.exit accordingly). Replace the existing node -e string in the check
invocation that references constants.nodeVersion to perform the explicit
exit-code check rather than using assert.
- Use process.exit() instead of assert() for node version check (not a global in Node v22) - Drop fixed --name from docker create to avoid rerun/parallel collisions - Make home dir root-owned so sandbox user cannot rename/delete .openclaw - Ensure .nemoclaw state dir stays writable by sandbox user Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
I attempted to port this branch across the JS→TS migration and merge the latest Please start with: git fetch origin
git merge origin/main
npx tsx scripts/ts-migration-assist.ts --base origin/main --write
npm run build:cli
npm run typecheck:cli
npm run lint
npm test |
Author
|
G'day @cv, thanks for taking a look! I think there may be a small misunderstanding about the scope of this PR — it doesn't modify any JavaScript or TypeScript source code, so the JS→TS migration steps shouldn't apply here. This PR adds Nix flake infrastructure for NemoClaw, providing:
All changes live under Happy to answer any questions or walk through the Nix setup! |
wscurran
added
the
status: rebase
Merge 420 upstream commits into nix branch. Conflicts resolved: - bin/lib/onboard.js: accept deletion (replaced by agent-onboard.js) - scripts/nemoclaw-start.sh: accept upstream version (security hardening) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Major upstream changes: CLI rewritten from JS monolith to compiled TypeScript (src/ → dist/), Python orchestrator deleted and rewritten in TS, gateway user added for gosu privilege separation, Dockerfile split into base + overlay. Nix packaging updates: - package.nix: three-phase build (plugin + CLI + assembly) - source-filter.nix: add cliSrc filter for root src/ + bin/ - constants.nix: add gatewayUser for privilege separation - container.nix: add gateway user/group, gosu, run as root - container-test.nix: 31 checks (gosu, gateway user, root uid) - shell.nix: remove ruff/pyright (no Python source), add build:cli - README.md: document three-phase build, gosu, dual npm hashes Container test results: 31/31 passed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 9 functional checks beyond structural verification: - nemoclaw CLI: --version, --help output - openclaw CLI: --version - Plugin: JS module loads without error - Blueprint: content readable, policies dir present - Privilege separation: gosu drops to sandbox (uid 1000) and gateway (uid 999) - CLI dist: compiled nemoclaw.js exists Container test results: 40/40 passed (31 structural + 9 functional) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Update smoke test count: 27 → 40 (31 structural + 9 functional) - Document new runtime env vars (model override, context window, etc.) - Add privilege separation section (gosu, gateway/sandbox users) - Update design decisions: remove ruff, add gosu explanation - Update file reference descriptions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add nix/container-scan.nix with trivy vulnerability scanning and dockle best-practice linting. Fix DKL-LI-0003 by removing unnecessary Dockerfiles from the package (CLI reads them from project root, not installed path). Show all findings transparently — only suppress CIS-DI-0010 (.env from upstream openclaw dependency). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Thanks for the reply and attention to the PR — great to see all the activity and innovation occurring on the project! I've updated the PR to track the latest 420+ upstream commits, including the TypeScript CLI migration, gosu privilege separation, and gateway user changes. I've also added more testing, including container security analysis via trivy and dockle. Container Test Results (40/40 passed)Security Scan (clean)What's next?Given how powerful Nix is for reproducible infrastructure, we could easily extend this further — for example, adding NixOS microVM-based automated tests that spin up a full isolated environment for end-to-end testing, or other CI/CD integrations. Happy to help get any of that set up if it would be useful to the project! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
nix/source-filter.nix (1)
41-46: Optional: Simplify blueprintSrc filter.The filter
_path: _type: trueis effectively a no-op. If no filtering is needed, you could uselib.cleanSourcedirectly or add a brief comment explaining why a pass-through filter is intentional here.Simplified alternative
# Just the nemoclaw-blueprint/ directory (blueprint.yaml + policies only) - blueprintSrc = lib.cleanSourceWith { - src = root + "/nemoclaw-blueprint"; - filter = _path: _type: true; - name = "nemoclaw-blueprint-source"; - }; + # No filtering needed — include all files in the blueprint directory + blueprintSrc = lib.cleanSource (root + "/nemoclaw-blueprint");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/source-filter.nix` around lines 41 - 46, The filter passed to lib.cleanSourceWith in blueprintSrc (filter = _path: _type: true) is a no-op; simplify by using lib.cleanSource instead of lib.cleanSourceWith (preserving src = root + "/nemoclaw-blueprint" and name = "nemoclaw-blueprint-source"), or if you intentionally want a pass-through keep lib.cleanSourceWith but replace the filter with a short clarifying comment referencing blueprintSrc and filter to explain why no filtering is required; update the code around blueprintSrc and lib.cleanSourceWith/lib.cleanSource accordingly.nix/package.nix (1)
118-120: Consider guarding against empty blueprint directory.The glob pattern
nemoclaw-blueprint/*will cause the command to fail if the directory is empty. While unlikely in practice, a defensive approach would be:Defensive alternative
# Blueprint (blueprint.yaml + policies only) mkdir -p $out/lib/nemoclaw-blueprint - cp -r nemoclaw-blueprint/* $out/lib/nemoclaw-blueprint/ + cp -r nemoclaw-blueprint/. $out/lib/nemoclaw-blueprint/Using
/.instead of/*copies directory contents including hidden files and handles empty directories gracefully.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/package.nix` around lines 118 - 120, The cp invocation that uses the glob nemoclaw-blueprint/* will fail when the source directory is empty; change the copy to use the directory dot form so hidden files are included and empty dirs are handled: replace cp -r nemoclaw-blueprint/* $out/lib/nemoclaw-blueprint/ with cp -r nemoclaw-blueprint/. $out/lib/nemoclaw-blueprint/ (keeping the existing mkdir -p $out/lib/nemoclaw-blueprint) so the Blueprint copy is robust against empty or dotfile-only source directories.nix/container-scan.nix (1)
48-62: Comment and code behavior diverge slightly.The comment on line 56 suggests filtering specific table characters (
│ ├ └ ┌), but the code skips all lines after "Report Summary" is encountered. This works but the comment is misleading. Consider updating the comment to match the actual behavior:Align comment with implementation
- # Skip summary table lines (they start with │ or ├ or └ or ┌) + # Skip everything after the summary header if $in_summary; then continue fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/container-scan.nix` around lines 48 - 62, The comment claims we only skip summary table lines starting with characters like │ ├ └ ┌, but the code (the while loop using in_summary=true) actually skips every line after "Report Summary"; update the inline comment that precedes the conditional (the line reading "# Skip summary table lines (they start with │ or ├ or └ or ┌)") to accurately state that all lines after "Report Summary" are skipped, or alternatively change the logic in the loop to only skip lines that match the table-row prefixes (inspect the while loop and the in_summary handling to implement the chosen behavior).nix/container-test.nix (1)
163-164: Path derivation is fragile but functional.The sed-based path manipulation (
sed 's|/bin/nemoclaw|/lib/dist/nemoclaw.js|') assumes a specific layout. If the wrapper structure changes, this check will silently break. Consider adding a comment explaining the expected layout or using a more direct path check if constants expose it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nix/container-test.nix` around lines 163 - 164, The check "CLI dist/nemoclaw.js exists" uses a fragile sed substitution on the output of command -v nemoclaw to derive the dist path; update this by either (a) replacing the sed-based derivation with a more robust resolution (e.g. compute the wrapper realpath and join with ../lib/dist/nemoclaw.js via dirname/realpath logic or invoke the CLI to print its dist location), or (b) at minimum add a clear comment above the check describing the exact wrapper layout you expect (that the executable lives in /bin/nemoclaw and the compiled output is at /lib/dist/nemoclaw.js) so future maintainers know the assumption; reference the check "CLI dist/nemoclaw.js exists" and the run_in invocation that currently contains the sed expression when making the change.flake.nix (1)
37-43: Hardcodedopenclawversion may cause breakage on nixpkgs updates.The version string
"openclaw-2026.3.12"must match the exact package name in nixpkgs. When nixpkgs updates openclaw to a new version, this will fail with an "insecure package not in permitted list" error.Consider adding a comment explaining the maintenance requirement, or dynamically deriving the version:
Option: Document or derive version
# openclaw is an autonomous AI agent marked insecure in nixpkgs because # it can execute arbitrary code. Acceptable here: NemoClaw is itself an # openclaw extension that requires the CLI, and the container runs sandboxed. + # NOTE: Update this version when nixpkgs updates the openclaw package. config.permittedInsecurePackages = [ "openclaw-2026.3.12" ];Or derive dynamically (though this requires evaluating pkgs first, which complicates the structure):
config.permittedInsecurePackages = [ "${pkgs.openclaw.name}" ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 37 - 43, The hardcoded entry "openclaw-2026.3.12" in config.permittedInsecurePackages will break when nixpkgs updates; either replace the literal with a dynamic reference derived from the evaluated package name (use pkgs.openclaw.name when you have pkgs available in this flake evaluation) or, if you prefer to keep a literal, add an explicit comment next to config.permittedInsecurePackages explaining the maintenance burden and instructing maintainers to update the string whenever pkgs.openclaw changes; locate the permittedInsecurePackages setting and update it to use pkgs.openclaw.name or add the maintenance comment accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@.claude/profiles/personal/projects/-home-das-Downloads-NemoClaw/memory/user_australian.md:
- Around line 1-7: The file with "name: Australian greeting preference" is a
local Claude profile that was accidentally committed; remove it from the repo
and stop tracking it, add the .claude directory to .gitignore to prevent future
commits, and commit the changes; if the file contains sensitive data that must
be purged from history, run a history-rewriting tool (e.g., BFG or git
filter-repo) to remove it from past commits and then force-push the cleaned
branch.
In `@nix/container-scan.nix`:
- Around line 73-84: The current dockle invocation uses "--exit-code 0" which
forces a zero exit and makes the if/else with EXIT_CODE unreachable; decide
whether dockle should be informational or should fail the build: for failing
behavior change "--exit-code 0" to "--exit-code 1" so dockle returns non-zero on
findings and the else branch setting EXIT_CODE=1 can trigger, or for
informational behavior remove the if/else entirely and run the dockle command
directly (keep the existing flags like --accept-key and --accept-file) and
replace the conditional echoes accordingly; update references to EXIT_CODE only
if you keep the failing conditional.
---
Nitpick comments:
In `@flake.nix`:
- Around line 37-43: The hardcoded entry "openclaw-2026.3.12" in
config.permittedInsecurePackages will break when nixpkgs updates; either replace
the literal with a dynamic reference derived from the evaluated package name
(use pkgs.openclaw.name when you have pkgs available in this flake evaluation)
or, if you prefer to keep a literal, add an explicit comment next to
config.permittedInsecurePackages explaining the maintenance burden and
instructing maintainers to update the string whenever pkgs.openclaw changes;
locate the permittedInsecurePackages setting and update it to use
pkgs.openclaw.name or add the maintenance comment accordingly.
In `@nix/container-scan.nix`:
- Around line 48-62: The comment claims we only skip summary table lines
starting with characters like │ ├ └ ┌, but the code (the while loop using
in_summary=true) actually skips every line after "Report Summary"; update the
inline comment that precedes the conditional (the line reading "# Skip summary
table lines (they start with │ or ├ or └ or ┌)") to accurately state that all
lines after "Report Summary" are skipped, or alternatively change the logic in
the loop to only skip lines that match the table-row prefixes (inspect the while
loop and the in_summary handling to implement the chosen behavior).
In `@nix/container-test.nix`:
- Around line 163-164: The check "CLI dist/nemoclaw.js exists" uses a fragile
sed substitution on the output of command -v nemoclaw to derive the dist path;
update this by either (a) replacing the sed-based derivation with a more robust
resolution (e.g. compute the wrapper realpath and join with
../lib/dist/nemoclaw.js via dirname/realpath logic or invoke the CLI to print
its dist location), or (b) at minimum add a clear comment above the check
describing the exact wrapper layout you expect (that the executable lives in
/bin/nemoclaw and the compiled output is at /lib/dist/nemoclaw.js) so future
maintainers know the assumption; reference the check "CLI dist/nemoclaw.js
exists" and the run_in invocation that currently contains the sed expression
when making the change.
In `@nix/package.nix`:
- Around line 118-120: The cp invocation that uses the glob nemoclaw-blueprint/*
will fail when the source directory is empty; change the copy to use the
directory dot form so hidden files are included and empty dirs are handled:
replace cp -r nemoclaw-blueprint/* $out/lib/nemoclaw-blueprint/ with cp -r
nemoclaw-blueprint/. $out/lib/nemoclaw-blueprint/ (keeping the existing mkdir -p
$out/lib/nemoclaw-blueprint) so the Blueprint copy is robust against empty or
dotfile-only source directories.
In `@nix/source-filter.nix`:
- Around line 41-46: The filter passed to lib.cleanSourceWith in blueprintSrc
(filter = _path: _type: true) is a no-op; simplify by using lib.cleanSource
instead of lib.cleanSourceWith (preserving src = root + "/nemoclaw-blueprint"
and name = "nemoclaw-blueprint-source"), or if you intentionally want a
pass-through keep lib.cleanSourceWith but replace the filter with a short
clarifying comment referencing blueprintSrc and filter to explain why no
filtering is required; update the code around blueprintSrc and
lib.cleanSourceWith/lib.cleanSource accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e475143c-56b3-4961-82a6-36db21b6c3f3
📒 Files selected for processing (11)
.claude/profiles/personal/projects/-home-das-Downloads-NemoClaw/memory/user_australian.md.gitignoreflake.nixnix/README.mdnix/constants.nixnix/container-scan.nixnix/container-test.nixnix/container.nixnix/package.nixnix/shell.nixnix/source-filter.nix
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- nix/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- nix/shell.nix
- nix/container.nix
- nix/constants.nix
Comment on lines
+1
to
+7
| --- | ||
| name: Australian greeting preference | ||
| description: User is Australian and prefers "G'day" over "Hi" in messages | ||
| type: user | ||
| --- | ||
|
|
||
| User is Australian. Prefers "G'day" as a greeting rather than "Hi" when drafting messages or comments on their behalf. |
Contributor
There was a problem hiding this comment.
Remove accidentally committed personal configuration file.
This file appears to be a local Claude profile configuration that was accidentally staged. The path contains user-specific directory structure (-home-das-Downloads-NemoClaw) and personal preferences that shouldn't be part of the repository.
Consider adding .claude/ to .gitignore to prevent future accidental commits of local IDE/tool configurations.
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.claude/profiles/personal/projects/-home-das-Downloads-NemoClaw/memory/user_australian.md
around lines 1 - 7, The file with "name: Australian greeting preference" is a
local Claude profile that was accidentally committed; remove it from the repo
and stop tracking it, add the .claude directory to .gitignore to prevent future
commits, and commit the changes; if the file contains sensitive data that must
be purged from history, run a history-rewriting tool (e.g., BFG or git
filter-repo) to remove it from past commits and then force-push the cleaned
branch.
Comment on lines
+73
to
+84
| if dockle \ | ||
| --input "$IMAGE_TARBALL" \ | ||
| --exit-code 0 \ | ||
| --accept-key "CIS-DI-0010" \ | ||
| --accept-file ".env" ; then | ||
| echo "" | ||
| echo "dockle: scan complete (see above for findings)" | ||
| else | ||
| echo "" | ||
| echo "dockle: scan failed" | ||
| EXIT_CODE=1 | ||
| fi |
Contributor
There was a problem hiding this comment.
The --exit-code 0 flag makes the conditional unreachable.
With --exit-code 0, dockle always returns success (exit code 0) regardless of findings. The else branch (lines 80-83) setting EXIT_CODE=1 will never execute.
If you want to fail on dockle findings, use --exit-code 1 (or omit the flag). If findings should be informational only (like trivy), remove the conditional:
Option A: Make dockle informational (consistent with trivy)
if dockle \
--input "$IMAGE_TARBALL" \
--exit-code 0 \
--accept-key "CIS-DI-0010" \
- --accept-file ".env" ; then
- echo ""
- echo "dockle: scan complete (see above for findings)"
- else
- echo ""
- echo "dockle: scan failed"
- EXIT_CODE=1
- fi
+ --accept-file ".env"
+ echo ""
+ echo "dockle: scan complete (see above for findings)"Option B: Fail on dockle findings
- if dockle \
+ if ! dockle \
--input "$IMAGE_TARBALL" \
- --exit-code 0 \
+ --exit-code 1 \
--accept-key "CIS-DI-0010" \
--accept-file ".env" ; then
echo ""
- echo "dockle: scan complete (see above for findings)"
- else
- echo ""
echo "dockle: scan failed"
EXIT_CODE=1
fi
+ echo ""
+ echo "dockle: scan complete"📝 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.
Suggested change
| if dockle \ | |
| --input "$IMAGE_TARBALL" \ | |
| --exit-code 0 \ | |
| --accept-key "CIS-DI-0010" \ | |
| --accept-file ".env" ; then | |
| echo "" | |
| echo "dockle: scan complete (see above for findings)" | |
| else | |
| echo "" | |
| echo "dockle: scan failed" | |
| EXIT_CODE=1 | |
| fi | |
| dockle \ | |
| --input "$IMAGE_TARBALL" \ | |
| --exit-code 0 \ | |
| --accept-key "CIS-DI-0010" \ | |
| --accept-file ".env" | |
| echo "" | |
| echo "dockle: scan complete (see above for findings)" |
Suggested change
| if dockle \ | |
| --input "$IMAGE_TARBALL" \ | |
| --exit-code 0 \ | |
| --accept-key "CIS-DI-0010" \ | |
| --accept-file ".env" ; then | |
| echo "" | |
| echo "dockle: scan complete (see above for findings)" | |
| else | |
| echo "" | |
| echo "dockle: scan failed" | |
| EXIT_CODE=1 | |
| fi | |
| if ! dockle \ | |
| --input "$IMAGE_TARBALL" \ | |
| --exit-code 1 \ | |
| --accept-key "CIS-DI-0010" \ | |
| --accept-file ".env" ; then | |
| echo "" | |
| echo "dockle: scan failed" | |
| EXIT_CODE=1 | |
| fi | |
| echo "" | |
| echo "dockle: scan complete" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nix/container-scan.nix` around lines 73 - 84, The current dockle invocation
uses "--exit-code 0" which forces a zero exit and makes the if/else with
EXIT_CODE unreachable; decide whether dockle should be informational or should
fail the build: for failing behavior change "--exit-code 0" to "--exit-code 1"
so dockle returns non-zero on findings and the else branch setting EXIT_CODE=1
can trigger, or for informational behavior remove the if/else entirely and run
the dockle command directly (keep the existing flags like --accept-key and
--accept-file) and replace the conditional echoes accordingly; update references
to EXIT_CODE only if you keep the failing conditional.
cv
reviewed
Apr 15, 2026
Author
There was a problem hiding this comment.
he he he! Indeed. Ooops!
- Remove accidentally committed .claude/profiles/ memory file and add .claude/profiles/ to .gitignore to prevent future leaks - Add gnugrep to container-test.nix runtimeInputs (check_output uses grep) - Fix dockle --exit-code: use 1 so WARN/FATAL findings fail the scan, while INFO findings remain acceptable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wscurran
removed
the
status: rebase
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Test plan
nix developenters dev shell with all toolsnix buildproduces NemoClaw packagenix build .#containerproduces OCI image (~1.1 GiB compressed)nix run .#container-testpasses all 27 checksnix build .#docsproduces Sphinx HTML (best-effort)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores