feat: add feature merging support#12
Draft
nozaq wants to merge 6 commits into
Draft
Conversation
Dev Container Features contribute properties (containerEnv, mounts, capAdd, securityOpt, privileged, init, customizations, and lifecycle hooks) that the tooling merges into the effective configuration per the specification's merge logic, so issues a Feature introduces are invisible when only the raw devcontainer.json is linted. Add an opt-in -merge-features flag (and a "mergeFeatures" config member) that fetches every referenced Feature - OCI artifacts with anonymous pull, HTTP(S) tarballs, and local paths - resolves dependsOn recursively, merges the contributed properties into the parsed syntax tree, and lints the result. Merged-in nodes carry the byte offset of the referencing "features" key, so findings on them point at the Feature reference and the usual ignore comments apply. A Feature that cannot be fetched is a runtime error (exit code 2). Fetched metadata is cached in memory for the duration of the run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrZdqcow5W3yq4uGMMPS8D
nozaq
changed the title
nozaq
changed the title
Resolves the conflict from #13's Options/Config separation refactor: MergeFeatures now merges through the new mergeConfig function (as a boolean OR, since either the -merge-features flag or the config file's "mergeFeatures" member can enable it) instead of the removed manual merge in run(). runLint now reads cfg.MergeFeatures rather than opts.MergeFeatures, since mergeConfig already folds the flag into cfg.
…tures Previously -merge-features and the config file's "mergeFeatures" member merged as a boolean OR, so the CLI flag could only enable merging, never disable it. Track whether -merge-features was explicitly passed (via flag.Visit) so mergeConfig can distinguish "not given" (defer to the config file) from "given as false" (override "mergeFeatures": true), matching how -platform already overrides the config file's "platforms" member. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrZdqcow5W3yq4uGMMPS8D
Move the "-merge-features overrides mergeFeatures" and "-platform overrides platforms" precedence notes out of their per-flag sections and into a single general statement in the Config file section, instead of repeating the rule for each flag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrZdqcow5W3yq4uGMMPS8D
Resolves the conflict from #16's os.Root-based path traversal hardening in linter.go/linter_test.go: - linter/linter_test.go: PR #16 rewrote this file to use the internal "package linter" test convention (dropping the linter_test/rules imports); the Transform tests added on this branch (flagRule, TestLintTransform) are adapted to that convention, unqualifying references to the now-local package. - cmd/decolint/main.go, linter/linter.go: auto-merged cleanly; runLint's SetTransform wiring for -merge-features is unaffected, since lintPath/LintDir's os.Root confinement only changes how the main devcontainer.json/Feature/Template files are discovered and read, not the *Context passed to Transform. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrZdqcow5W3yq4uGMMPS8D
#16 routed the main devcontainer.json/Feature/Template file discovery through os.Root to prevent path traversal, but local Feature references resolved during -merge-features (a relative path under "features") still went through plain os.ReadFile/os.Stat, bypassing that protection. Thread the same os.Root boundary through: linter.Context gains Dir (the os.Root LintDir already confines the file to) and FileDir (the file's own directory relative to it), populated by lintConfig and left zero when Lint is called directly on in-memory content (as existing tests do). feature.Fetch/Merge now take that (dir, fileDir) pair instead of a bare baseDir string, and resolve local references by joining fileDir with the reference and reading it through dir, so a reference cannot escape the same boundary the referencing devcontainer.json was itself read through. Note this changes local Feature semantics slightly: a Feature stored outside the confining directory (e.g. a project-root sibling of .devcontainer, referenced from .devcontainer/devcontainer.json via "../") no longer resolves, even though it did before #16. This mirrors the boundary #16 already established for the file discovery its own comment anticipated extending to Feature resolution. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrZdqcow5W3yq4uGMMPS8D
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
2 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.
This PR adds support for merging Dev Container Features into the effective configuration before linting, enabling decolint to catch security and configuration issues that only manifest after Features are applied.
Summary
Implements the Dev Container specification's merge logic for Features, allowing decolint to lint the final effective configuration rather than just the raw
devcontainer.json. This is critical for security analysis since Features can contribute privileged mode, capability additions, mounts, and other properties that may violate linting rules.Key Changes
New Feature Package (
feature/)ref.go: Parses Feature references (OCI, tarball, local paths)fetch.go: Fetches Feature metadata with caching and size limitsmetadata.go: Parsesdevcontainer-feature.jsondeclarationsoci.go: Implements OCI distribution API for registry-hosted Featurestarball.go: Extracts Features from tar/gzip archivesmerge.go: Orchestrates Feature resolution and installation orderingapply.go: Implements the specification's merge logic for all property typesCLI Integration
-merge-featuresflag to enable Feature merging during lintingmergeFeaturesconfig file option for persistent enablementLinter Enhancement
Transformcallback support in linter for pre-rule mutationsImplementation Details
dependsOn,installsAfter, andoverrideFeatureInstallOrderhttps://claude.ai/code/session_01UrZdqcow5W3yq4uGMMPS8D