Define evaluator-sandbox image contract and docker:build - #93
Conversation
📝 WalkthroughWalkthroughThe PR defines the evaluator sandbox image contract, documents its runtime assumptions, adds a ChangesEvaluator sandbox image
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
0908e55 to
996bbce
Compare
Document the MVP image contract, align the Dockerfile comments with runtime network-none isolation, add rake docker:build to tag evaluator-sandbox with the gem version and latest, and cover the contract with unit tests (Closes #87).
CI failed on Layout/TrailingEmptyLines for the docker:build task block.
996bbce to
5e088c8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/docker.md`:
- Around line 23-27: Update the Docker build instruction near the
repository-root command to apply only to a repository checkout, or ensure the
package includes the Rakefile and its required lib files before claiming
installed packages can run it. Do not imply that any install containing the
packaged context supports bundle exec rake docker:build.
- Line 14: Update the Docker documentation’s “Preinstalled” row to explicitly
list the required supported shell, preferably /bin/sh, instead of relying on the
vague “minimal Debian tools” wording. Add or update the corresponding contract
assertion to verify that this exact shell is available.
- Line 13: Update the Base entry in the Docker documentation to reference the
approved immutable digest for the desired ruby:3.4-bookworm image instead of the
mutable tag, preserving the documented Ruby version and architecture alignment.
In `@lib/skill_bench/execution/docker/Dockerfile`:
- Line 5: Correct the misspelled capability name in the Dockerfile contract
comment from CHOUN to CHOWN, matching Sandbox#start_container and docs/docker.md
without changing the documented flags.
In `@test/evaluator/docker_image_contract_test.rb`:
- Around line 27-32: Update test_version_tag_format_matches_gem_version to
exercise the docker:build task rather than constructing the expected tag from
SkillBench constants. Stub the Docker command, invoke docker:build with its real
arguments, and assert that both the versioned and latest tags are generated
correctly.
- Around line 34-40: Update test_gemspec_packages_docker_docs_optional to
reflect the complete Docker context, renaming it accordingly and asserting that
spec.files includes Dockerfile, .dockerignore, and docs/docker.md. Keep the
existing gemspec loading setup unchanged.
🪄 Autofix
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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a4adcc87-e4c5-4660-beb3-dc50a0648978
📒 Files selected for processing (4)
Rakefiledocs/docker.mdlib/skill_bench/execution/docker/Dockerfiletest/evaluator/docker_image_contract_test.rb
| |------|--------| | ||
| | Image name | `evaluator-sandbox` (`Constants::Sandbox::DOCKER_IMAGE_NAME`) | | ||
| | Tags | `evaluator-sandbox:<SkillBench::VERSION>` and `evaluator-sandbox:latest` | | ||
| | Base | `ruby:3.4-bookworm` (aligns with CI Ruby 3.4; 3.3 hosts still run evals via host Ruby for the orchestrator) | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
docker buildx imagetools inspect ruby:3.4-bookwormRepository: igmarin/ruby-skill-bench
Length of output: 206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked files matching Docker/docs\n'
git ls-files | rg '(^|/)(Dockerfile|docker\.md|\.dockerignore$|compose)' || true
printf '\ndocs/docker.md context\n'
if [ -f docs/docker.md ]; then
nl -ba docs/docker.md | sed -n '1,80p'
fi
printf '\nDockerfiles\n'
fd -a 'Dockerfile$' . | while read -r f; do
echo "--- $f"
nl -ba "$f" | sed -n '1,120p'
doneRepository: igmarin/ruby-skill-bench
Length of output: 367
🌐 Web query:
Docker Hub ruby 3.4-bookworm image multi-architecture digest FROM ruby:3.4-bookworm mutable tag documentation reproducibility
💡 Result:
The ruby:3.4-bookworm tag is a mutable reference, meaning it can point to different container images over time as the base image is rebuilt (e.g., for security patches) [1][2][3]. Because it is mutable, it does not guarantee reproducibility [1][4][3]. To achieve reproducibility, you must reference the image by its immutable manifest digest (e.g., ruby:3.4-bookworm@sha256:e46ac468c9e87ff9209709c5b73ac689688ee82839e2a18af33d14d3577d1340) [1][5][3]. Key points regarding your query: 1. Multi-Architecture Digests: The ruby:3.4-bookworm tag points to a manifest list (index), which contains platform-specific digests for architectures like amd64, arm64v8, and others [6][7][8]. While the tag is mutable, the specific manifest digest (e.g., sha256:e46ac468...) uniquely identifies the content [9][10][5]. You can view the current digest for this tag on Docker Hub via the image's "Tag" or "Layers" view [9][10][6]. 2. Tag Mutability: Official image tags like 3.4-bookworm are intended to provide convenience by pointing to the latest version of that specific Ruby/OS combination [7][1]. However, this implies they will drift [1][2]. Reliance on these tags in production can lead to environment drift where different builds or nodes inadvertently use different versions of the base image [5][3]. 3. Reproducibility Best Practices: - Always pin to a digest (repository@sha256:...) in production deployment manifests and build definitions to ensure identical image usage across environments [1][5][3]. - If using tags is required for ergonomics, acknowledge the mutability risk and consider enforcing tag immutability policies in your private registry if you re-tag or mirror images [2][5]. - For complete reproducibility, combine base image digest pinning with locked application dependencies (e.g., lock files) and controlled system package versions [3]. Current data confirms that the ruby:3.4-bookworm tag is regularly updated by the maintainers [9][10], reinforcing the necessity of using digests for stable, reproducible deployments [5][3].
Citations:
- 1: https://developers.redhat.com/articles/2025/01/28/how-name-version-and-reference-container-images
- 2: https://www.docker.com/blog/docker-best-practices-using-tags-and-labels-to-manage-docker-image-sprawl/
- 3: https://oneuptime.com/blog/post/2026-02-08-how-to-build-reproducible-docker-images-with-locked-dependencies/view
- 4: https://www.pulumi.com/docs/iac/operations/container-image-reproducibility/
- 5: https://cr0x.net/en/docker-image-tag-rule/
- 6: https://hub.docker.com/_/ruby/tags
- 7: https://hub.docker.com/_/ruby?tab=tags
- 8: https://github.com/docker-library/official-images/blob/master/library/ruby
- 9: https://hub.docker.com/layers/library/ruby/3.4-bookworm/images/sha256-05499d2830c9683c31cb59de3c642f8ac3e13886faa260432184073be5896a63
- 10: https://hub.docker.com/layers/library/ruby/3.4-bookworm/images/sha256-3cde37c1582543c4f03b7f9edd5504649e8164de36c3aff2f1a9bcdee809e59b
Pin the Docker base image to a digest.
ruby:3.4-bookworm is a mutable tag, so evaluator-sandbox:<SkillBench::VERSION> can be rebuilt from a different base image over time. Use an approved immutable registry digest that resolves to the desired architecture and update it through a deliberate process.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/docker.md` at line 13, Update the Base entry in the Docker documentation
to reference the approved immutable digest for the desired ruby:3.4-bookworm
image instead of the mutable tag, preserving the documented Ruby version and
architecture alignment.
| | Image name | `evaluator-sandbox` (`Constants::Sandbox::DOCKER_IMAGE_NAME`) | | ||
| | Tags | `evaluator-sandbox:<SkillBench::VERSION>` and `evaluator-sandbox:latest` | | ||
| | Base | `ruby:3.4-bookworm` (aligns with CI Ruby 3.4; 3.3 hosts still run evals via host Ruby for the orchestrator) | | ||
| | Preinstalled | Ruby (from base), `git`, minimal Debian tools from base image | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Name the required shell explicitly.
The MVP contract requires a shell, but this row only says “minimal Debian tools from base image.” Name /bin/sh or the exact supported shell. Add a matching contract assertion.
Proposed documentation update
-| Preinstalled | Ruby (from base), `git`, minimal Debian tools from base image |
+| Preinstalled | Ruby (from base), `/bin/sh`, `git`, and minimal Debian tools from base image |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/docker.md` at line 14, Update the Docker documentation’s “Preinstalled”
row to explicitly list the required supported shell, preferably /bin/sh, instead
of relying on the vague “minimal Debian tools” wording. Add or update the
corresponding contract assertion to verify that this exact shell is available.
| From the repo root (or any install that includes the packaged context): | ||
|
|
||
| ```bash | ||
| bundle exec rake docker:build | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restrict this build instruction to a repository checkout, or package the task.
Rakefile Lines 69-75 load lib/... with require_relative. The supplied gemspec file list includes the Docker context and documentation, but not Rakefile. An installed package cannot run bundle exec rake docker:build from this paragraph as written.
Proposed documentation update
-From the repo root (or any install that includes the packaged context):
+From a repository checkout:📝 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.
| From the repo root (or any install that includes the packaged context): | |
| ```bash | |
| bundle exec rake docker:build | |
| ``` | |
| From a repository checkout: | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/docker.md` around lines 23 - 27, Update the Docker build instruction
near the repository-root command to apply only to a repository checkout, or
ensure the package includes the Rakefile and its required lib files before
claiming installed packages can run it. Do not imply that any install containing
the packaged context supports bundle exec rake docker:build.
| # | ||
| # Purpose: long-lived container for `docker exec` of allowlisted agent commands. | ||
| # Runtime flags (from Sandbox#start_container): --network none, non-root uid/gid, | ||
| # --security-opt no-new-privileges, --cap-drop ALL (+ CHOUN/DAC_OVERRIDE). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Correct the capability name.
The comment says CHOUN. Sandbox#start_container and docs/docker.md use CHOWN. Keep the Dockerfile contract comment aligned with the flags that docker run uses.
Proposed fix
-# --security-opt no-new-privileges, --cap-drop ALL (+ CHOUN/DAC_OVERRIDE).
+# --security-opt no-new-privileges, --cap-drop ALL (+ CHOWN/DAC_OVERRIDE).📝 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.
| # --security-opt no-new-privileges, --cap-drop ALL (+ CHOUN/DAC_OVERRIDE). | |
| # --security-opt no-new-privileges, --cap-drop ALL (+ CHOWN/DAC_OVERRIDE). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/skill_bench/execution/docker/Dockerfile` at line 5, Correct the
misspelled capability name in the Dockerfile contract comment from CHOUN to
CHOWN, matching Sandbox#start_container and docs/docker.md without changing the
documented flags.
| def test_version_tag_format_matches_gem_version | ||
| image = SkillBench::Constants::Sandbox::DOCKER_IMAGE_NAME | ||
| versioned = "#{image}:#{SkillBench::VERSION}" | ||
|
|
||
| assert_equal "evaluator-sandbox:#{SkillBench::VERSION}", versioned | ||
| end |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Exercise the build task in the tag test.
The test constructs versioned from the same constants that it asserts. It does not detect a missing version tag, a missing latest tag, or incorrect Rakefile arguments. Stub the Docker command while invoking docker:build, or extract and test a pure tag-building helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/evaluator/docker_image_contract_test.rb` around lines 27 - 32, Update
test_version_tag_format_matches_gem_version to exercise the docker:build task
rather than constructing the expected tag from SkillBench constants. Stub the
Docker command, invoke docker:build with its real arguments, and assert that
both the versioned and latest tags are generated correctly.
| def test_gemspec_packages_docker_docs_optional | ||
| # Dockerfile packaging is required (#86); docs/docker.md is repo docs. | ||
| gemspec_path = File.expand_path('../../ruby-skill-bench.gemspec', __dir__) | ||
| spec = Gem::Specification.load(gemspec_path) | ||
|
|
||
| assert_includes spec.files, 'docs/docker.md' | ||
| end |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Assert the complete packaged Docker context.
This test checks only docs/docker.md. The gemspec also packages Dockerfile and .dockerignore, but the test does not protect those files. Rename the test and assert all required context files.
Proposed test update
- def test_gemspec_packages_docker_docs_optional
- # Dockerfile packaging is required (`#86`); docs/docker.md is repo docs.
+ def test_gemspec_packages_docker_context_and_docs
+ # The Docker context and documentation are required by the package contract.
gemspec_path = File.expand_path('../../ruby-skill-bench.gemspec', __dir__)
spec = Gem::Specification.load(gemspec_path)
+ assert_includes spec.files, 'lib/skill_bench/execution/docker/Dockerfile'
+ assert_includes spec.files, 'lib/skill_bench/execution/docker/.dockerignore'
assert_includes spec.files, 'docs/docker.md'📝 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.
| def test_gemspec_packages_docker_docs_optional | |
| # Dockerfile packaging is required (#86); docs/docker.md is repo docs. | |
| gemspec_path = File.expand_path('../../ruby-skill-bench.gemspec', __dir__) | |
| spec = Gem::Specification.load(gemspec_path) | |
| assert_includes spec.files, 'docs/docker.md' | |
| end | |
| def test_gemspec_packages_docker_context_and_docs | |
| # The Docker context and documentation are required by the package contract. | |
| gemspec_path = File.expand_path('../../ruby-skill-bench.gemspec', __dir__) | |
| spec = Gem::Specification.load(gemspec_path) | |
| assert_includes spec.files, 'lib/skill_bench/execution/docker/Dockerfile' | |
| assert_includes spec.files, 'lib/skill_bench/execution/docker/.dockerignore' | |
| assert_includes spec.files, 'docs/docker.md' | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/evaluator/docker_image_contract_test.rb` around lines 34 - 40, Update
test_gemspec_packages_docker_docs_optional to reflect the complete Docker
context, renaming it accordingly and asserting that spec.files includes
Dockerfile, .dockerignore, and docs/docker.md. Keep the existing gemspec loading
setup unchanged.
Summary Prefer Docker isolation when context + daemon are available ensure_image builds only when versioned tag is missing (skip rebuild) Use evaluator-sandbox:<VERSION> for run; tag :latest on build container_id stays nil when Docker is unavailable (fail-closed host unchanged) Unit tests cover availability, skip-build, security flags, and run paths Closes #88 Base: #93 (image contract). Milestone: container-isolation-v1 Stack BE | Package Docker build context in gem (#86) #92 packaging (BE | Package Docker build context in gem #86) Define evaluator-sandbox image contract and docker:build #93 image contract (BE | Define evaluator-sandbox image contract + build #87) This PR activation (BE | Activate container lifecycle with safe fallbacks #88) BE | Live Docker integration tests (opt-in) #89 live tests (next) BE | CI job for container integration #90 CI BE | Docs + CHANGELOG for shipped container isolation #91 docs Pre-push Lifecycle unit tests green (9) Related sandbox / run_command / packaging tests green RuboCop + YARD coverage clean on touched files rs-guard: deepseek call hung; self-review: security flags preserved, fail-closed host, no allow_host default change Test plan lifecycle unit tests sandbox + packaging + run_command CI green
1 participant
Summary
docs/docker.md--network noneruntime modelrake docker:buildtaggingevaluator-sandbox:<VERSION>and:latestCloses #87
Base: stacks on #92 (packaging).
Milestone: container-isolation-v1
Stack
Test plan
bundle exec ruby -Itest test/evaluator/docker_image_contract_test.rbbundle exec rake docker:buildwhen Docker daemon availableSummary by CodeRabbit
New Features
latest.Documentation
Tests