Repository navigation
Environments: an inline build context for foreignDockerfile - #41
Merged
Merged
Conversation
…, so the spec's digest covers its bytes
There was a problem hiding this comment.
🟡 Changes recommended
Context validation bypasses allow generated Dockerfiles and resolved locks to be overwritten, while valid JSON-form and wildcard COPY sources are incorrectly rejected.
4 open issues
What changed in this PR
Adds inline build-context files for foreignDockerfile environments, enabling Dockerfile COPY instructions to use files stored with the environment version and covered by its spec digest.
Changes:
- Adds context-file models, validation, and a 1 MiB decoded-content limit while preserving digests when context is unset.
- Adds context-aware source checks and writes supplied files into the Datalayer build directory.
- Updates the schema, documentation, unit tests, and Docker integration coverage.
| File | Description |
|---|---|
tests/test_environment_foreign_contract_layer.py |
Tests context validation, digests, file writing, and image copying. |
schemas/environment-v1alpha1.json |
Defines inline context files and the optional context field. |
docs/docs/environments/specification.mdx |
Documents inline context support and its size limit. |
code_sandboxes/environments/spec.py |
Models and validates context files; omits unset context from serialization. |
code_sandboxes/environments/contract.py |
Adds context limits, reserved paths, and source-membership checks. |
code_sandboxes/environments/adapters/datalayer.py |
Writes context files and permissions before building. |
Review effort: Balanced
| Checked again here, against the folder itself, because this is the step | ||
| that writes to disk: nothing validated elsewhere is taken on trust. | ||
| """ | ||
| dockerfile = environment.spec.build.dockerfile |
| findings.append(SpecFinding(field, f"`{item.path}` is not base64")) | ||
| continue | ||
| entries.append(BuildContextEntry(path=item.path, size_bytes=size)) | ||
| if item.path in RESERVED_CONTEXT_PATHS: |
| name = source.strip("/").removeprefix("./") | ||
| if name in ("", "."): | ||
| return bool(context) | ||
| return any(path == name or path.startswith(f"{name}/") for path in context) |
| continue | ||
| sources = [t for t in tokens if not t.startswith("--")][:-1] | ||
| local = [s for s in sources if not re.match(r"^(https?|git)://|^git@", s, re.IGNORECASE)] | ||
| local = [s for s in local if not _in_context(s, context)] |
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
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.


A
foreignDockerfilecould notCOPYfrom a build context. This takes one inline, inbuild.dockerfile.context, the waydependencyFilealready keeps its file in the spec.validate_build_context: no absolute path, no.., no symlink (a file here has no way to be one). Paths the build itself writes are reserved.write_build_contextchecks each target again against the folder, because that is the step that writes.COPYis allowed only for what the context holds. With none, it is refused as before.contextis left out of the dump when unset, so no existing spec's digest moves.dockerfilesources refuse it.Tested: unit tests, and a real build copying and running a context file, with
DATALAYER_ENVIRONMENT_DOCKER_TESTS=1. A local image stood in for the approved base.