You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds DockerExecutor to @sverka/runtime-docker for secure, digest-verified container runs, and switches @sverka/core and @sverka/ir to deterministic SHA-256 op- IDs. Also hardens @sverka/runtime-host to treat process spawn failures as runtime errors.
Bug Fixes
Image verification uses RepoDigests, pulls if missing, applies timeouts, and raises clear ImageDigestError.
Artifact collection validates paths under request.artifactDir, rejects absolute destinations, uses workspace-relative sources, and blocks traversal with specific ContainerPolicyError codes.
Cache manager prevents path traversal, preserves input directory structure using the workspace root, skips missing outputs on collect, fixes ownership handling, and mounts under /cache.
Policy checks canonicalize paths/env for Docker socket detection; default non-root runAs is "1000:1000"; tightened secret denylist to avoid PUBLIC_KEY false positives and added specific codes for blocked socket and undeclared secrets.
Log truncation preserves a visible notice and caps output while streaming via the Docker CLI seam.
Planner preserves __matrixCombo, throws on unknown dependency IDs, suppresses finalize rejections, and unifies skipped outcomes; @sverka/ir re-exports computeOperationId, validates operation kinds, recomputes plan IDs ignoring identity fields, and accepts extra top-level fields in ID calculation.
Host executor treats spawn failures as runtime errors and avoids double-resolving on process error.
Written for commit c0e424f. Summary will update on new commits.
CodeAnt-AI Description
Add secure Docker execution and content-addressed operation IDs
What Changed
Adds Docker execution with digest-pinned images, read-only containers, dropped capabilities, configurable CPU and memory limits, network controls, required timeouts, logs, and artifact collection
Blocks Docker socket access and undeclared secret-like environment variables
Adds persistent cache preparation and output collection, plus clear errors for policy violations and image digest mismatches
Replaces readable operation IDs with deterministic op- IDs based on operation content, including matrix values and commands; dependency references and duplicate detection use these IDs
Exposes shared canonical serialization and operation ID helpers, with tests covering matrix uniqueness, dependency ordering, serialization rules, and Docker behavior
Impact
✅ Sandboxed Docker execution ✅ Reproducible operation and matrix IDs ✅ Blocked Docker socket and undeclared secret access
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Added Docker-based execution with security policies, timeouts, logging, artifact collection, and image digest verification.
Added persistent caching for execution inputs and outputs.
Added structured Docker errors and configuration options.
Exported the Docker runtime APIs and content-addressed operation ID utility.
Improvements
Operation IDs are now deterministic, content-addressed, and consistently generated across packages.
Matrix workflows support expanded combinations without the previous limit.
Documentation
Added comprehensive Docker runtime implementation and configuration documentation.
Tests
Expanded coverage for workflows, IDs, Docker execution, caching, security, artifacts, and integration scenarios.
Walkthrough
This PR adds content-addressed operation IDs shared by Core and IR, updates planning and DAG tests, and implements the Docker runtime package with execution policies, digest verification, caching, artifact handling, error types, CLI timeouts, exports, and tests.
Changes
Core planning and Docker runtime
Layer / File(s)
Summary
Runtime Docker contract and implementation plan engdocs/architecture/wave-05-runtime-docker-plan.md, specs/04-runtime-docker/spec.md
Operation IDs now use canonical JSON and SHA-256 content addressing. Planning resolves aliases, dependencies, and matrix context with the new IDs.
Docker process and image foundation packages/runtime-docker/src/config.ts, packages/runtime-docker/src/errors.ts, packages/runtime-docker/src/internal/docker-cli.ts, packages/runtime-docker/src/image.ts, packages/runtime-docker/src/__tests__/errors.test.ts, packages/runtime-docker/src/__tests__/image.test.ts
Adds Docker configuration, error types, timeout-aware CLI execution, and image digest verification with pull-and-recheck behavior.
Adds policy-compliant container execution, cache preparation and collection, artifact handling, public exports, integration coverage, and the updated lint command.
Running ultrareview automatically — Because this PR lands the security-sensitive Docker executor (isolation flags, credential allowlisting, image digest pinning, mount policy, cache/artifact paths), where a subtle bug could leak secrets, escape the sandbox, or break every run, a deeper review is warranted.. I'll post findings when complete.
• Implement DockerExecutor for running pinned images with strict container security policy.
• Add mockable docker CLI seam plus unit/integration coverage with daemon-guarded tests.
• Update spec/plan docs to match runtime contract and build/test commands.
The following are alternative approaches to this PR:
1. Use Docker Engine API (dockerode) instead of docker CLI
➕ Avoids CLI parsing and shelling out; structured errors/streams
➕ Potentially better performance and richer lifecycle controls
➖ Adds a heavier dependency and API/daemon compatibility surface
➖ Harder to mock cleanly without standing up API fakes
➖ May require additional auth/connection handling across environments
2. Reuse runtime-host spawn utilities directly (shared process runner)
➕ Reduces duplicated timeout/log-capture logic across executors
➕ Centralizes process execution hardening in one place
➖ Tighter coupling between runtime-host and runtime-docker packages
➖ Still needs a test seam boundary; shared code may become a layering issue
Recommendation: Current approach (docker CLI wrapper as the single seam + pure arg/env builders) is a good fit for portability and unit-testability. Consider extracting shared spawn/timeout/log-truncation helpers later if runtime-host and runtime-docker diverge or bugs appear in both.
• Introduces a CacheManager interface and DockerCacheManager implementation to prepare cache directories keyed by operation and to collect output files back into a persistent cacheDir.
• Adds the executor-wide configuration interface including dockerPath/dockerHost overrides, runAs, cacheDir, and maxLogBytes; clarifies workspace and artifactDir are per-request.
docker-executor.tsImplement DockerExecutor with policy enforcement, logs, and artifacts+293/-0
Implement DockerExecutor with policy enforcement, logs, and artifacts
• Implements the Executor contract: validates docker operations, enforces mandatory timeout and digest presence, builds docker run args/env with network and secret/socket policies, executes via runDocker, truncates logs, and collects declared artifacts without altering run status on artifact errors.
• Defines DockerExecutorError and two specializations: ImageDigestError for digest mismatches and ContainerPolicyError for policy violations, each with stable codes and optional context.
• Implements digest verification by inspecting local image ID, pulling when missing, and throwing ImageDigestError with expected/actual digest context on mismatch.
docker-cli.tsAdd mockable docker CLI runner with timeout enforcement+92/-0
Add mockable docker CLI runner with timeout enforcement
• Introduces the single side-effect seam for spawning the docker CLI, capturing stdout/stderr, returning exit codes, applying a timeout with SIGTERM/SIGKILL grace, and supporting DOCKER_HOST/dockerPath overrides.
cache.test.tsAdd DockerCacheManager unit tests+79/-0
Add DockerCacheManager unit tests
• Adds coverage for cache directory creation, copying declared inputs, collecting outputs back to the persistent cacheDir, and restore-from-cache behavior when sources are missing.
errors.test.tsAdd error hierarchy tests for runtime-docker+57/-0
Add error hierarchy tests for runtime-docker
• Verifies error base fields (name/code/context) and subclass behavior for ImageDigestError and ContainerPolicyError including instanceof relationships.
• Adds opt-in integration tests (guarded by SVERKA_DOCKER) that run simple commands in busybox and validate success/failure outcomes using a real Docker daemon.
wave-05-runtime-docker-plan.mdAdd Wave 5 implementation plan for runtime-docker+321/-0
Add Wave 5 implementation plan for runtime-docker
• Introduces a detailed build-and-test plan for the Docker executor, including spec amendments, file layout, TDD sequencing, and edge-case policy rules (secrets allowlist, socket denial, timeouts, artifacts).
spec.mdAmend runtime-docker spec to match built runtime request contract+17/-17
Amend runtime-docker spec to match built runtime request contract
• Updates DockerExecutorConfig and command examples to use request.workspace/request.artifactDir, clarifies credentials/value sourcing via request.*, and corrects test commands to 'bun run test'.
All issues have specific fixes provided. The test coverage is excellent (58 tests), but these logic errors and security vulnerabilities must be addressed before the code is production-ready.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The pull request implements the @sverka/runtime-docker package but is currently not up to standards due to a significant volume of quality issues and critical logic errors.
The most pressing issues involve the DockerExecutor implementation: it includes an unsupported --timeout flag that will cause all container executions to fail, and it misses a mandatory architectural step to call verifyImageDigest before execution. Furthermore, a security risk was identified in artifact collection where absolute paths could allow the exfiltration of host files.
Logic gaps in DockerCacheManager will cause file collisions and failures with directories, as the implementation flattens input structures and uses non-recursive copy operations.
About this PR
Artifact collection allows absolute paths, which permits the exfiltration of arbitrary host files readable by the executor process, potentially bypassing the intended workspace boundary. Artifact paths should be strictly validated as relative to the workspace.
The DockerCacheManager.prepare implementation flattens input files into the root of the cache directory, which will cause collisions for different files that share a basename (e.g., 'src/index.ts' and 'test/index.ts').
Test suggestions
Found recommended test scenario: DockerExecutor.canExecute identifies docker operations and rejects others
Found recommended test scenario: buildDockerArgs correctly constructs CLI flags for security policy and resource limits
Found recommended test scenario: Network policy correctly maps to Docker network modes
Missing recommended test scenario: Execution fails with ContainerPolicyError if timeout or image digest is missing
Found recommended test scenario: Environment variables are filtered by credential declarations and secret-like patterns are blocked
Found recommended test scenario: Docker socket references in mounts or env vars trigger a policy violation error
Found recommended test scenario: verifyImageDigest correctly handles inspect failures by pulling the image
Found recommended test scenario: Logs exceeding maxLogBytes are truncated and appended with a notice
Found recommended test scenario: Artifacts are copied from the workspace to the artifactDir after execution
Missing recommended test scenario: DockerCacheManager preserves output structure and correctly restores cached inputs
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing recommended test scenario: Execution fails with ContainerPolicyError if timeout or image digest is missing
2. Missing recommended test scenario: DockerCacheManager preserves output structure and correctly restores cached inputs
1. verifyImageDigest never called before container run✓ Resolved🐞 Bug⛨ Security
Description
DockerExecutor.execute() only checks that op.executor.imageDigest is *present*, but never calls
verifyImageDigest() to confirm the locally available/pulled image actually matches that declared
digest before spawning the container via runDocker(). This defeats the supply-chain integrity
guarantee described in the spec and plan ('If the pulled image's digest does not match the declared
digest, ImageDigestError is raised... The container is not started'), allowing a tampered or stale
image tagged with the same name to run unchecked.
image.ts exports verifyImageDigest (docker inspect + pull + digest comparison) and it is exercised
only from image.test.ts. docker-executor.ts imports only ContainerPolicyError and runDocker from
internal/docker-cli.js — verifyImageDigest is never imported or invoked anywhere in execute(). The
plan explicitly lists digest mismatch as a pre-spawn check ('Digest mismatch — ImageDigestError with
both digests in context', 'Missing digest — ContainerPolicyError before spawn') implying
verification, not just presence, must occur before runDocker.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
DockerExecutor.execute() checks only that `op.executor.imageDigest` is defined, but never calls the `verifyImageDigest` function to confirm the actual local/pulled image digest matches the declared digest before running the container. This bypasses the intended supply-chain integrity check.
## Issue Context
`verifyImageDigest(image, expectedDigest, config)` is implemented in `image.ts` and does `docker inspect`/`docker pull` + digest comparison, throwing `ImageDigestError` on mismatch. It is currently only invoked from its own unit tests, never from the executor's `execute()` path.
## Fix Focus Areas
- packages/runtime-docker/src/docker-executor.ts[161-181]
- packages/runtime-docker/src/image.ts[13-46]
Call `verifyImageDigest(op.executor.image, op.executor.imageDigest, this.config)` after the presence check (step 3) and before `buildDockerArgs`/`runDocker` (steps 4-5), letting `ImageDigestError` propagate. Add/extend a docker-executor test asserting `runDocker` is never invoked when the mocked digest check fails.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Artifacts can read host files✓ Resolved🐞 Bug⛨ Security
Description
Absolute artifact paths are copied directly by the host executor process, allowing a plan to collect
any host-readable file rather than only files produced in its workspace. This bypasses the container
boundary and can exfiltrate host credentials or configuration.
+ for (const artifact of op.artifacts) {+ const src = isAbsolute(artifact.path)+ ? artifact.path+ : join(workspace, artifact.path);
Evidence
The artifact declaration exposes an unrestricted string path, IR validation has no artifact
confinement rule, and collectArtifacts uses absolute paths directly in host-side copyFile.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Absolute artifact paths are accepted as host filesystem sources without checking that they reside under the execution workspace.
## Issue Context
Require artifact sources to be relative, resolve them against the canonical workspace, and reject traversal or symlink resolution outside that workspace before reading.
## Fix Focus Areas
- packages/runtime-docker/src/docker-executor.ts[275-283]
- packages/ir/src/plan.ts[90-94]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. SIGKILL fallback never runs✓ Resolved🐞 Bug☼ Reliability
Description
After a successful SIGTERM, child.killed indicates signal delivery rather than process exit, so
the grace callback skips SIGKILL even when the Docker CLI remains alive. Because the promise
resolves only on error or close, such executions can hang indefinitely despite their timeout.
+ setTimeout(() => {+ if (!child.killed) {+ child.kill("SIGKILL");+ }
Evidence
The fallback is gated on !child.killed, while Node documents that this property only records
successful signal delivery and does not indicate termination.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The timeout escalation checks `child.killed`, which becomes true after successful signal delivery and does not mean the process exited.
## Issue Context
Track an `exited`/`closed` flag set by the exit or close event, send `SIGKILL` after the grace period unless that flag is set, and clear the grace timer when the child closes.
## Fix Focus Areas
- packages/runtime-docker/src/internal/docker-cli.ts[55-63]
- packages/runtime-docker/src/internal/docker-cli.ts[72-90]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Resolved credential values are serialized as --env KEY=value Docker arguments, exposing them to
host process inspection, wrappers, and command-line audit logging. Pass secret values through a
mechanism that does not place them in child-process argv.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Container credential values are embedded directly in the Docker CLI process arguments.
## Issue Context
`buildEnv` includes resolved `request.credentials`, and `runDocker` passes the generated argument array directly to `spawn`. Use a secure environment-file or another Docker/API mechanism that avoids argv disclosure, with restrictive permissions and reliable cleanup if a temporary file is used.
## Fix Focus Areas
- packages/runtime-docker/src/docker-executor.ts[80-84]
- packages/runtime-docker/src/docker-executor.ts[114-120]
- packages/runtime-docker/src/internal/docker-cli.ts[50-53]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
buildDockerArgs unconditionally passes --timeout to docker run, but that command has no such
option, so every real execution fails before the container starts. The timeout is already enforced
by runDocker, so this invalid CLI argument should be removed.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`DockerExecutor.buildDockerArgs` adds an unsupported `--timeout` option to `docker run`, causing all real Docker executions to fail during argument parsing.
## Issue Context
The executor already passes `timeoutSeconds` to `runDocker`, which owns host-side timeout enforcement. Docker supports `--stop-timeout` for container stop behavior, not a runtime duration limit.
## Fix Focus Areas
- packages/runtime-docker/src/docker-executor.ts[54-55]
- packages/runtime-docker/src/internal/docker-cli.ts[55-63]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
artifact.name or the fallback artifact.path is joined to artifactDir without a containment
check, so values containing .. can create or overwrite files outside the artifact directory.
Validate the resolved destination before creating directories or copying data.
The destination is constructed from unvalidated artifact metadata with join, which normalizes ..
but does not sandbox the result, and the executor immediately creates parent directories and writes
the file.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Artifact destination metadata can traverse outside `request.artifactDir` through parent-directory components or absolute-like values.
## Issue Context
Resolve the destination beneath a canonical artifact root and reject any result outside it before `mkdir` or `copyFile`; apply the check to both `artifact.name` and fallback `artifact.path`, and account for symlinked path components.
## Fix Focus Areas
- packages/runtime-docker/src/docker-executor.ts[279-283]
- packages/ir/src/plan.ts[90-94]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
verifyImageDigest compares the image configuration .Id with the declared manifest/repository
digest used by image@digest, so valid pinned images are rejected. Verification must inspect and
normalize the matching RepoDigests entry or otherwise resolve the manifest digest.
+ let inspectResult = await runDocker(+ ["inspect", "--format={{.Id}}", image],+ opts,
Evidence
The IR defines imageDigest for container image pinning and the executor uses it in image@digest;
Docker's API documentation states that image Id is a configuration digest and differs from
manifest digests in RepoDigests.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`verifyImageDigest` reads Docker image `.Id`, which is a configuration digest, and compares it with the operation's repository/manifest digest.
## Issue Context
The declared digest is used in an `image@sha256:...` reference. Docker documents `.Id` and `RepoDigests` as different digest types.
## Fix Focus Areas
- packages/runtime-docker/src/image.ts[28-40]
- packages/runtime-docker/src/docker-executor.ts[253-259]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
DockerCacheManager.prepare() computes the destination as `join(target, relative(dirname(input),
input))`, which always evaluates to just the input's basename (since relative(dirname(x), x) ===
basename(x)), so any two declared inputs from different subdirectories but the same filename
silently overwrite each other in the cache and lose their directory structure — inconsistent with
collect(), which preserves full relative paths via relative(sourceDir, output).
+ for (const input of inputs) {+ const dest = join(target, relative(dirname(input), input));+ await mkdir(dirname(dest), { recursive: true });
Evidence
For any path input, relative(dirname(input), input) reduces to the basename of input, e.g.
relative('/a/b', '/a/b/c.txt') === 'c.txt'. Declared cache inputs like 'src/a/util.ts' and
'src/b/util.ts' both map to '<cacheDir>/<key>/util.ts', so the second copy silently clobbers the
first. This contradicts the class docstring claiming prepare/collect 'preserv[e] relative path
structure', and diverges from collect()'s own path handling (line 43: `relative(sourceDir,
output)`), which does preserve subpaths.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`DockerCacheManager.prepare()` computes the cache destination path using `relative(dirname(input), input)`, which always yields just the basename of the input file, discarding directory structure and causing filename collisions between inputs from different subdirectories.
## Issue Context
`collect()` in the same file preserves full relative paths via `relative(sourceDir, output)`. `prepare()` should use an analogous, well-defined root (e.g. accept a `sourceDir`/workspace root parameter, or require `inputs` be passed as paths relative to a known root) so the two methods use a consistent addressing scheme and avoid basename collisions.
## Fix Focus Areas
- packages/runtime-docker/src/cache.ts[23-39]
Change `prepare` to take (or derive) a base directory and compute `dest = join(target, relative(baseDir, input))` instead of `relative(dirname(input), input)`. Add a test with two inputs sharing a basename in different subdirectories to confirm both are preserved distinctly.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
docker-executor.ts only includes exitCode in the ExecuteResult when result.exitCode >= 0, but
internal/docker-cli.ts deliberately returns exitCode: -1 for spawn errors and null-close events;
this filter silently discards that diagnostic sentinel, unlike the sibling runtime-host package
which always surfaces the code via code !== null.
internal/docker-cli.ts sets exitCode: -1 explicitly on child.on('error', ...) (line 77) and on
close with a null code (code ?? -1, line 87) to signal an abnormal spawn/exit.
docker-executor.ts's result.exitCode >= 0 guard (lines 193, 204) strips this sentinel from the
returned ExecuteResult, whereas runtime-host's host-executor.ts uses code !== null and always
keeps the code (including negative values) in the result, so the same class of failure is
diagnosable in one package but not the other.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`docker-executor.ts` only attaches `exitCode` to the `ExecuteResult` when it is `>= 0`, discarding the `-1` sentinel that `internal/docker-cli.ts` uses to signal spawn errors or abnormal termination, reducing diagnosability of Docker CLI launch failures.
## Issue Context
The sibling `runtime-host` package's `host-executor.ts` always includes the exit code (via `code !== null`) even when negative, keeping the same information visible to callers/schedulers.
## Fix Focus Areas
- packages/runtime-docker/src/docker-executor.ts[188-212]
Replace the `result.exitCode >= 0` guard with a check appropriate to distinguish 'no code available' from 'negative sentinel', e.g. always include `exitCode: result.exitCode`, or align semantics/tests with host-executor's pattern.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Core's assignId/assignIds used human-readable string derivation
(kind:name-or-command-or-index with collision counter) and passed
user spec.id through as-is, violating ADR-006. Now uses SHA-256
content-addressed ids: op-<64 hex> via node:crypto.createHash over
canonical JSON of {kind, name, context}, with spec.id folded into
hash context as userId.
- Created packages/core/src/internal/canonical.ts (computeOperationId)
- Rewrote packages/core/src/internal/ids.ts + plan.ts for SHA-256
- Exported computeOperationId from core public index.ts
- Fixed all tests to assert op-<64hex> format (90 core tests pass)
- Added cross-consistency test: core === ir computeOperationId (87 ir tests)
- Full monorepo: 16 projects test+typecheck+build green
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🤖 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 `@packages/core/src/__tests__/matrix.test.ts`:
- Around line 28-39: Update the multi-dimension cartesian-product test to use
two values for both node and os, then assert four operations with distinct ids
and the expected matrix environment values. Restore coverage for duplicate
matrix values, asserting the duplicate-id CompositionError from assignIds, and
for values with identical text but different types (1, "1", true), asserting
they receive distinct ids via canonicalStringify.
In `@packages/core/src/internal/plan.ts`:
- Line 489: Update the dependency handling in resolveEdges, detectCycles, and
topoSort so missing byId entries are not silently skipped. Replace the
byId.has(dep) guards with an explicit invariant failure that raises
CompositionError, while preserving normal traversal and ordering for resolved
dependencies.
- Around line 164-177: Preserve each matrix child’s __matrixCombo when
flattenArtifacts rebuilds nodes via makeNodeWith, including artifact-predecessor
and propagated-join paths. Thread the combination through flattenArtifacts,
assignIds, and contextFor, or retain it as typed node metadata, so contextFor
includes matrix values and generated operation IDs remain distinct.
In `@packages/ir/src/__tests__/core-consistency.test.ts`:
- Around line 5-27: Update the ADR-006 consistency test to stop comparing
coreComputeOperationId with irComputeOperationId, since the latter is
re-exported from core and cannot detect drift. Replace the self-comparison cases
with assertions against committed, precomputed golden hash values for each
input, and revise the test comment to describe the shared algorithm and
golden-value regression coverage accurately.
In `@packages/runtime-docker/src/__tests__/integration.test.ts`:
- Around line 13-16: Update the integration test setup around the busybox image
configuration and both test cases so no dummy digest is used. Require
SVERKA_BUSYBOX_DIGEST whenever SVERKA_DOCKER enables the suite, or skip the
entire suite unless both environment variables are set; preserve normal behavior
when integration tests are disabled.
In `@packages/runtime-docker/src/cache.ts`:
- Around line 8-12: Update the CacheManager.collect contract and its
implementations/call sites to receive or otherwise retain the cache key used by
prepare. Ensure collected outputs are written beneath the same <cacheDir>/<key>
directory returned by prepare, while preserving relative output paths within
that key-scoped directory.
- Around line 23-46: Update prepare and collect to resolve each target path and
reject any candidate that escapes cacheDir; validate the resolved prepare target
derived from key and each collect destination derived from sourceDir/output
before creating directories or copying files. Preserve valid nested paths, and
add traversal tests covering escaping key values and outputs outside sourceDir.
In `@packages/runtime-docker/src/config.ts`:
- Around line 12-13: Make the runAs property in the configuration type optional
to match its documented default, then apply "1000:1000" within DockerExecutor
whenever the value is absent. Preserve explicitly provided runAs values and
update the executor’s container configuration path accordingly.
In `@packages/runtime-docker/src/docker-executor.ts`:
- Around line 272-275: Update truncateLogs so truncated output, including
TRUNCATION_NOTICE, never exceeds maxLogBytes. Reserve the notice length when
calculating the slice boundary, and handle limits smaller than the notice by
returning only the allowed number of characters without exceeding the configured
limit.
- Around line 143-149: Update DockerExecutor.execute and its cache flow to use
the configured DockerExecutorConfig.cacheDir consistently instead of mounting
request.cacheDir. Establish a single cache ownership contract, call
DockerCacheManager.prepare() for the operation’s key before runContainer(), and
call DockerCacheManager.collect() for declared outputs after execution while
preserving result finalization.
- Line 289: Validate the destination derived in the artifact-copy flow around
dest before mkdir() or copyFile() runs: resolve artifactDir and the
artifact.name ?? artifact.path target, then reject targets outside the resolved
artifactDir, including traversal in both named and unnamed artifacts. Add tests
covering traversal through artifact.name and artifact.path while preserving
valid destinations.
In `@packages/runtime-docker/src/errors.ts`:
- Around line 26-29: Update ContainerPolicyError to accept a specific policy
error code and pass that code to DockerExecutorError instead of always using
CONTAINER_POLICY_VIOLATION. Revise every ContainerPolicyError call site to
provide the appropriate code, including MISSING_TIMEOUT, MISSING_DIGEST,
UNDECLARED_SECRET, and DOCKER_SOCKET_DENIED, and update tests to assert those
codes.
In `@packages/runtime-docker/src/image.ts`:
- Around line 28-36: Update the image inspection flow around runDocker so a pull
occurs only when the inspect result confirms the image is absent, and return
immediately for timedOut results. Validate the pull result before re-inspecting,
stopping on pull failure, and include Docker’s error output in the raised error
context instead of allowing an empty digest mismatch.
- Around line 28-44: Update the image verification flow around verifyImageDigest
to compare the registry manifest digest from RepoDigests, or inspect the
immutable image@expectedDigest reference, instead of comparing docker inspect’s
.Id configuration digest. Ensure DockerExecutor invokes verifyImageDigest on the
execution path before using the image, while preserving the existing
pull-and-reinspect behavior and ImageDigestError reporting.
In `@packages/runtime-docker/src/internal/docker-cli.ts`:
- Around line 65-70: Update DockerExecutor.runDocker and its stdout/stderr data
handlers to accept the configured maxLogBytes limit, append output only while
the accumulated byte count remains within that limit, and record that truncation
occurred once the limit is exceeded. Continue consuming both child streams after
truncation, and preserve the existing DockerExecutor.truncateLogs behavior for
final output handling.
- Around line 55-62: Update the timeout handling around the child process and
its "close" handler to track whether the child has actually closed, rather than
checking child.killed. Clear the grace-period timer when the close handler runs,
and send SIGKILL after GRACE_PERIOD_MS only when the tracked closed state
remains false.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 333ff9e5-c755-4078-8339-eba48ef0bc40
📥 Commits
Reviewing files that changed from the base of the PR and between 5908184 and 818a725.
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
Also applies to: 51-52, 75-95, 104-112, 128-130, 143-152, 165-173
packages/core/src/index.ts (1)
28-28: LGTM!
packages/core/src/internal/canonical.ts (1)
2-18: LGTM!
Also applies to: 68-70, 108-113, 144-144
packages/core/src/internal/ids.ts (1)
1-28: LGTM!
packages/core/src/internal/plan.ts (4)
327-357: LGTM!
402-442: LGTM!
4-4: LGTM!
Also applies to: 29-41, 88-97
371-390: 🎯 Functional Correctness
Do not expand contextFor to include the full operation spec.
The documented operation-ID contract uses matrix values, userId, command, and args. Other operation fields are included in the emitted operation and therefore affect computePlanId; they are not inputs to computeOperationId.
> Likely an incorrect or invalid review comment.
packages/ir/src/internal/canonical.ts (1)
1-8: LGTM!
packages/ir/src/ids.ts (1)
2-2: 🗄️ Data Integrity & Integration
No dependency change is required.packages/ir/package.json declares @sverka/core under dependencies, so published consumers can resolve the runtime import.
- core/project.json: remove --passWithNoTests from test target
- core/ids.test.ts: add mixed-type context test
- core/plan.ts: suppress finalize rejection and extract outcomeForSkipped
- ir/validate.ts: recompute id by omitting identity fields, remove validateAcyclic early return, validate operation kind
- ir/validate.test.ts: remove unused plan constant, add extra top-level field test
- ir/core-consistency.test.ts: use OperationKind type and remove as never casts
Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
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
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.
codeant-ai
Bot
added
the
size:XXL
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
User description
Summary
Test plan
Stacked on #5
Generated with Devin
Summary by cubic
Adds
DockerExecutorto@sverka/runtime-dockerfor secure, digest-verified container runs, and switches@sverka/coreand@sverka/irto deterministic SHA-256op-IDs. Also hardens@sverka/runtime-hostto treat process spawn failures as runtime errors.ImageDigestError.request.artifactDir, rejects absolute destinations, uses workspace-relative sources, and blocks traversal with specificContainerPolicyErrorcodes./cache.runAsis "1000:1000"; tightened secret denylist to avoidPUBLIC_KEYfalse positives and added specific codes for blocked socket and undeclared secrets.__matrixCombo, throws on unknown dependency IDs, suppresses finalize rejections, and unifies skipped outcomes;@sverka/irre-exportscomputeOperationId, validates operation kinds, recomputes plan IDs ignoring identity fields, and accepts extra top-level fields in ID calculation.error.Written for commit c0e424f. Summary will update on new commits.
CodeAnt-AI Description
Add secure Docker execution and content-addressed operation IDs
What Changed
op-IDs based on operation content, including matrix values and commands; dependency references and duplicate detection use these IDsImpact
✅ Sandboxed Docker execution✅ Reproducible operation and matrix IDs✅ Blocked Docker socket and undeclared secret access🔄 Retrigger CodeAnt AI Review
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.