From 3402f5556b1b592fb4896686bc653e6f92567f00 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 20 Sep 2026 21:40:52 -0300 Subject: [PATCH 1/4] test(oci): isolate build-auth tests from the host docker config ResolveBuildAuth resolves credentials through the docker config and, when that file is missing, through the platform credential helper. The tests passed no DOCKER_CONFIG, so on a developer machine they resolved the real ghcr.io/docker.io credentials, asserted against whatever the host was logged into, and printed that credential in the failure message. Point them at a DOCKER_CONFIG holding credentials for an unrelated registry: present, so the platform-helper fallback stays out, and irrelevant to every registry under test. Co-Authored-By: Claude Opus 5 (1M context) --- internal/oci/buildauth_test.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/oci/buildauth_test.go b/internal/oci/buildauth_test.go index 97157d6..eff4d3f 100644 --- a/internal/oci/buildauth_test.go +++ b/internal/oci/buildauth_test.go @@ -26,10 +26,27 @@ func readConfigAuths(t *testing.T, dir string) map[string]dockerConfigAuth { return cf.Auths } +// isolatedDockerConfig returns a DOCKER_CONFIG directory holding a config.json +// with credentials for an unrelated registry. Credential resolution must not fall +// back to the developer's own ~/.docker/config.json or to a platform credential +// helper: both would resolve real credentials for ghcr.io/docker.io and make +// these tests assert against whatever the host happens to be logged into (and +// print that credential on failure). +func isolatedDockerConfig(t *testing.T) string { + t.Helper() + dir := t.TempDir() + cfg := `{"auths":{"unrelated.example.com":{"auth":"dXNlcjpwYXNz"}}}` + if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(cfg), 0o600); err != nil { + t.Fatalf("write docker config: %v", err) + } + return dir +} + func TestResolveBuildAuthFromOCIEnvAndGitHubToken(t *testing.T) { env := map[string]string{ "DEVCONTAINERS_OCI_AUTH": "myreg.example.com|user|s3cr3t", "GITHUB_TOKEN": "ghtok", + "DOCKER_CONFIG": isolatedDockerConfig(t), } dir, cleanup, ok, err := ResolveBuildAuth(env, []string{"myreg.example.com", "ghcr.io", "docker.io"}, log.Null) if err != nil || !ok { @@ -56,7 +73,8 @@ func TestResolveBuildAuthFromOCIEnvAndGitHubToken(t *testing.T) { } func TestResolveBuildAuthNoCredsIsNoop(t *testing.T) { - dir, cleanup, ok, err := ResolveBuildAuth(map[string]string{}, []string{"private.example.com", "docker.io"}, log.Null) + env := map[string]string{"DOCKER_CONFIG": isolatedDockerConfig(t)} + dir, cleanup, ok, err := ResolveBuildAuth(env, []string{"private.example.com", "docker.io"}, log.Null) defer cleanup() if ok || dir != "" || err != nil { t.Fatalf("expected no-op: ok=%v dir=%q err=%v", ok, dir, err) @@ -64,7 +82,7 @@ func TestResolveBuildAuthNoCredsIsNoop(t *testing.T) { } func TestResolveBuildAuthCleanupRemovesDir(t *testing.T) { - env := map[string]string{"DEVCONTAINERS_OCI_AUTH": "reg.example.com|u|p"} + env := map[string]string{"DEVCONTAINERS_OCI_AUTH": "reg.example.com|u|p", "DOCKER_CONFIG": isolatedDockerConfig(t)} dir, cleanup, ok, err := ResolveBuildAuth(env, []string{"reg.example.com"}, log.Null) if !ok || err != nil { t.Fatalf("ok=%v err=%v", ok, err) @@ -79,7 +97,7 @@ func TestResolveBuildAuthCleanupRemovesDir(t *testing.T) { } func TestResolveBuildAuthDedupesRegistries(t *testing.T) { - env := map[string]string{"DEVCONTAINERS_OCI_AUTH": "reg.example.com|u|p"} + env := map[string]string{"DEVCONTAINERS_OCI_AUTH": "reg.example.com|u|p", "DOCKER_CONFIG": isolatedDockerConfig(t)} dir, cleanup, ok, _ := ResolveBuildAuth(env, []string{"reg.example.com", "reg.example.com", ""}, log.Null) if !ok { t.Fatal("expected ok") From 21593920604394a70399ad137f04f6e07246c42f Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 20 Sep 2026 21:41:00 -0300 Subject: [PATCH 2/4] feat(oci): support OCI auth hardening from reference v0.89.0 Bump the pinned reference CLI to v0.89.0 and implement the observable surface it added (PR devcontainers/cli#1278): - `--oci-auth-hardening` and `--allow-cross-origin-auth-host` as global flags, with the same validation the oracle applies: the allow list requires hardening, and each entry is a '=' pair of bare authorities. - `ociAuthDiagnostics` in the `up`, `build` and `read-configuration` output, reporting what hardening would change. - Bearer realms pinned to the registry authority or a trusted auth host (including the built-in Docker Hub and GitLab mappings), and token endpoints refused a redirect, when hardening is on. - `scheme` on the feature ref in `read-configuration` output, and the generated feature Dockerfiles defaulting the base-image ARG to `scratch` rather than `placeholder`. The policy is built once per invocation and carried on the command context, so every OCI client of a command shares its settings and feeds the same diagnostics. `exec` parses its own flags and therefore applies the validation itself; `features test` re-invokes this binary and forwards the flags to the `up` it spawns. The hardening is enforced in a transport above oras-go, which already refuses to forward credentials to a cross-origin challenge. That makes hardening-off stricter here than upstream; the divergence is documented and the diagnostics still report what hardening would change. TestOracleFlagCoverage only inspected per-command options, so the two new global flags went unnoticed; it now checks the oracle's global options as well, and TestFlagInventoryParity pins them to the root command. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/go-cli.yml | 4 +- README.md | 2 +- docs/DIVERGENCES.md | 12 +- docs/parity/cli-flags-inventory.yaml | 20 +- docs/parity/parity-matrix.yaml | 56 ++++ internal/cli/build.go | 7 +- internal/cli/collection_commands.go | 3 +- internal/cli/exec.go | 6 + internal/cli/feature_install.go | 11 +- internal/cli/feature_install_test.go | 2 +- internal/cli/features_info.go | 8 +- internal/cli/features_resolve_deps.go | 3 +- internal/cli/features_test_runner.go | 29 +- internal/cli/features_test_runner_test.go | 6 +- internal/cli/flag_inventory_test.go | 28 +- internal/cli/ociauth.go | 97 ++++++ internal/cli/ociauth_test.go | 242 +++++++++++++++ internal/cli/oracle_flag_coverage_test.go | 47 +++ internal/cli/outdated.go | 9 +- internal/cli/read_configuration.go | 8 +- internal/cli/root.go | 12 + internal/cli/templates_apply.go | 3 +- internal/cli/templates_metadata.go | 7 +- internal/cli/up.go | 6 +- internal/imagemeta/extend.go | 4 +- internal/oci/authpolicy.go | 360 ++++++++++++++++++++++ internal/oci/authpolicy_test.go | 309 +++++++++++++++++++ internal/oci/client.go | 12 + internal/oci/orasclient.go | 19 ++ internal/oci/ref.go | 15 + reference | 2 +- 31 files changed, 1295 insertions(+), 54 deletions(-) create mode 100644 internal/cli/ociauth.go create mode 100644 internal/cli/ociauth_test.go create mode 100644 internal/oci/authpolicy.go create mode 100644 internal/oci/authpolicy_test.go diff --git a/.github/workflows/go-cli.yml b/.github/workflows/go-cli.yml index 0ce934e..50370c5 100644 --- a/.github/workflows/go-cli.yml +++ b/.github/workflows/go-cli.yml @@ -109,7 +109,7 @@ jobs: - uses: actions/upload-artifact@v7 if: always() with: - name: parity-contract-network-v0.88.0 + name: parity-contract-network-v0.89.0 path: artifacts/ if-no-files-found: error - uses: actions/upload-artifact@v7 @@ -222,7 +222,7 @@ jobs: - uses: actions/upload-artifact@v7 if: always() && steps.plan.outputs.run == 'true' with: - name: parity-runtime-v0.88.0-shard-${{ matrix.shard }} + name: parity-runtime-v0.89.0-shard-${{ matrix.shard }} path: artifacts/ if-no-files-found: error # daily only: covdata slice for the cross-lane merge (distinct name per shard). diff --git a/README.md b/README.md index 275842a..911550e 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ GitHub Releases are not supported targets. “Compatible” means compatibility this scope; it does not mean that every platform or historical upstream behavior is implemented. -A pinned official TypeScript CLI (`reference/`, currently v0.88.0) is the behavioral +A pinned official TypeScript CLI (`reference/`, currently v0.89.0) is the behavioral oracle. Roughly 200 cases run commands through both CLIs and compare exit status, normalized output, and relevant container or registry state. See the [parity matrix](docs/parity/parity-matrix.yaml) and diff --git a/docs/DIVERGENCES.md b/docs/DIVERGENCES.md index ad95cc6..7e43a4e 100644 --- a/docs/DIVERGENCES.md +++ b/docs/DIVERGENCES.md @@ -1,7 +1,7 @@ # Divergences, decisions & accepted limitations This CLI is validated for behavioral parity with the reference TypeScript -`@devcontainers/cli` (pinned at **v0.88.0**, see [`parity/`](parity/)). Where it +`@devcontainers/cli` (pinned at **v0.89.0**, see [`parity/`](parity/)). Where it deliberately differs, the difference is recorded here — this is the durable record of *intentional* departures from the oracle, not a backlog. User-facing additions are documented in [`go-only-features.md`](go-only-features.md). @@ -32,6 +32,16 @@ touches a compared surface, reflected in the parity matrix. - **`config.build.cacheFrom`** is honored (wired to `--cache-from` after the flag's values) — matching `singleContainer.ts`. Upstream defines the field; this is a parity fix, noted here because it was previously a dead field. +- **OCI auth hardening is enforced through `oras-go`, which is stricter by default.** + `--oci-auth-hardening` and `--allow-cross-origin-auth-host` behave as documented + upstream (bearer realms pinned to the registry authority or a trusted auth host, + token endpoints may not redirect), and `ociAuthDiagnostics` reports the same three + flags in `up`/`build`/`read-configuration`. The difference is what happens + **without** the flag: the reference CLI still forwards registry credentials to a + challenge that arrives from another origin, while `oras-go` never does + (GHSA-vh4v-2xq2-g5cg). Hardening off is therefore already safe here; the + diagnostics still report what hardening *would* change, so the flag remains a + faithful compatibility probe. - **`BUILDKIT_INLINE_CACHE=1`** is omitted when `--cache-to` is an inline exporter (`/type\s*=\s*inline/i`), matching TS `isBuildxCacheToInline` — a parity fix over the earlier unconditional build-arg. diff --git a/docs/parity/cli-flags-inventory.yaml b/docs/parity/cli-flags-inventory.yaml index ce83a8d..4e81a29 100644 --- a/docs/parity/cli-flags-inventory.yaml +++ b/docs/parity/cli-flags-inventory.yaml @@ -21,6 +21,20 @@ parser_configuration: strict: true demand_command: true +# ───────────────────────────────────────────────────────────────────────────── +# Global flags (yargs `.option(..., { global: true })`: accepted by every command) +# ───────────────────────────────────────────────────────────────────────────── +global_flags: + oci-auth-hardening: + type: boolean + default: false + description: "Restrict OCI bearer authentication realms, registry credential forwarding, and token redirects." + allow-cross-origin-auth-host: + type: string + array: true + description: "Allow an OCI registry to use a cross-origin HTTPS authentication host. Format: =. May be repeated." + notes: "Requires --oci-auth-hardening; each entry must be '=' of bare authorities." + # ───────────────────────────────────────────────────────────────────────────── # Global validations (shared by multiple commands) # ───────────────────────────────────────────────────────────────────────────── @@ -1140,17 +1154,17 @@ commands: # ============================================================================= json_output_envelopes: success_with_container: # up - fields: [outcome, containerId, remoteUser, remoteWorkspaceFolder, composeProjectName, configuration, mergedConfiguration] + fields: [outcome, containerId, remoteUser, remoteWorkspaceFolder, composeProjectName, configuration, mergedConfiguration, ociAuthDiagnostics] success_setup: # set-up fields: [outcome, configuration, mergedConfiguration] success_build: # build - fields: [outcome, imageName] + fields: [outcome, imageName, ociAuthDiagnostics] success_run_user_commands: # run-user-commands fields: [outcome, result] error: # all commands with outcome envelope fields: [outcome, message, description, containerId, disallowedFeatureId, didStopContainer, learnMoreUrl] read_configuration: # read-configuration (NO outcome envelope) - fields: [configuration, workspace, featuresConfiguration, mergedConfiguration] + fields: [configuration, workspace, featuresConfiguration, mergedConfiguration, ociAuthDiagnostics] # ============================================================================= # ENVIRONMENT VARIABLES READ diff --git a/docs/parity/parity-matrix.yaml b/docs/parity/parity-matrix.yaml index fd42278..480a43e 100644 --- a/docs/parity/parity-matrix.yaml +++ b/docs/parity/parity-matrix.yaml @@ -9,6 +9,7 @@ meta: scope: languages: [ts, go] commands: [features-info, read-configuration, exec, run-user-commands, set-up, up, build] + global_flags: [oci-auth-hardening, allow-cross-origin-auth-host] lanes: contract: description: "Parsing, required args, enums, error formats, output shape" @@ -491,6 +492,61 @@ initial_cases: class: format-validation current_status: match + - id: read-configuration.oci-auth-hardening-accepted + lane: contract + command: read-configuration + priority: p0 + docker_required: false + ts_cmd: "--oci-auth-hardening read-configuration --workspace-folder src/test/configs/image" + asserts: [exit_code, stdout_normalized] + class: global-flags + current_status: match + notes: "0.89 global flag: accepted before the command name and the output carries the ociAuthDiagnostics envelope field (all false when no registry is contacted)." + + - id: read-configuration.cross-origin-auth-host-requires-hardening + lane: contract + command: read-configuration + priority: p0 + docker_required: false + ts_cmd: "--allow-cross-origin-auth-host registry.example=auth.example read-configuration --workspace-folder src/test/configs/image" + asserts: [exit_code, stderr_normalized] + class: global-flags + current_status: match + notes: "0.89 yargs .check(): --allow-cross-origin-auth-host requires --oci-auth-hardening." + + - id: read-configuration.cross-origin-auth-host-invalid-pair + lane: contract + command: read-configuration + priority: p1 + docker_required: false + ts_cmd: "--oci-auth-hardening --allow-cross-origin-auth-host bad read-configuration --workspace-folder src/test/configs/image" + asserts: [exit_code, stderr_normalized] + class: global-flags + current_status: match + notes: "0.89: each entry must be '='." + + - id: read-configuration.cross-origin-auth-host-invalid-authority + lane: contract + command: read-configuration + priority: p1 + docker_required: false + ts_cmd: "--oci-auth-hardening --allow-cross-origin-auth-host a/b=c read-configuration --workspace-folder src/test/configs/image" + asserts: [exit_code, stderr_normalized] + class: global-flags + current_status: match + notes: "0.89: both sides of the mapping must be bare authorities." + + - id: read-configuration.global-options-consume-one-argument + lane: contract + command: read-configuration + priority: p1 + docker_required: false + ts_cmd: "--oci-auth-hardening --allow-cross-origin-auth-host registry.example=auth.example read-configuration --workspace-folder src/test/configs/image" + asserts: [exit_code, stdout_normalized] + class: global-flags + current_status: match + notes: "Mirrors the oracle's own 'Global options consume exactly one argument' test (src/test/cli.test.ts), asserted on read-configuration because yargs and cobra render --help differently: the repeatable flag must consume exactly one value and leave the subcommand intact." + - id: read-configuration.terminal-columns-implies-rows lane: contract command: read-configuration diff --git a/internal/cli/build.go b/internal/cli/build.go index 0f7ba28..1cbdbb2 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -163,6 +163,8 @@ func runBuild(ctx context.Context, out Output, opts *buildOpts) error { Format: opts.logFormat, Writer: os.Stderr, }) + // Route the OCI auth diagnostic lines at this command's logger. + ociAuthPolicy(ctx).SetLogger(logger) // Load config loadResult, err := config.LoadDevContainerConfig(workspaceFolder, configPath, "") @@ -243,8 +245,9 @@ func runBuild(ctx context.Context, out Output, opts *buildOpts) error { } return writeSuccessJSON(out, map[string]interface{}{ - "outcome": "success", - "imageName": imageNameResult, + "outcome": "success", + "imageName": imageNameResult, + "ociAuthDiagnostics": ociAuthDiagnostics(ctx), }) } diff --git a/internal/cli/collection_commands.go b/internal/cli/collection_commands.go index e01e1aa..9295922 100644 --- a/internal/cli/collection_commands.go +++ b/internal/cli/collection_commands.go @@ -238,6 +238,7 @@ func realFeaturesTestCmd() *cobra.Command { preserve, quiet, permitRandomization, + ociAuthGlobalArgs(cmd), ) if exitCode != 0 { return &coreerrors.ExitCodeError{Code: exitCode} @@ -438,7 +439,7 @@ func publishCollection(ctx context.Context, targetFolder, registry, namespace, c Format: "text", Writer: os.Stderr, }) - reg := oci.NewClient(logger, osEnvMap()) + reg := newOCIClient(ctx, logger) return publishCollectionWith(ctx, OSOutput(), reg, targetFolder, registry, namespace, collectionType, logLevelStr) } diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 5ce8251..6a262c7 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -62,6 +62,11 @@ func newExecCmd() *cobra.Command { if err := cmd.ParseFlags(flagArgs); err != nil { return err } + // DisableFlagParsing skips the root's PersistentPreRunE validation, so + // the global OCI auth flags are validated here, once parsed. + if err := applyOCIAuthPolicy(cmd); err != nil { + return err + } opts.workspaceFolder, _ = cmd.Flags().GetString("workspace-folder") opts.configPath, _ = cmd.Flags().GetString("config") @@ -433,6 +438,7 @@ func splitExecArgs(args []string) (flags []string, cmd []string) { "--default-user-env-probe": true, "--user-data-folder": true, "--terminal-columns": true, "--terminal-rows": true, "--log-file": true, "--terminal-log-file": true, + "--" + flagAllowCrossOriginAuthHos: true, } i := 0 diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index 5b5a203..c791d51 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -79,8 +79,8 @@ type fetchFeatureResult struct { // fetchFeatureSets fetches features and returns them in install order. reg is the // registry seam; pass nil for the default OCI client. -func fetchFeatureSets(logger log.Logger, reg oci.Registry, featuresCfg map[string]interface{}, featuresBasePath string, skipAutoMapping bool, lockfile *features.Lockfile) (*fetchFeatureResult, error) { - return fetchFeatureSetsWithOrder(logger, reg, featuresCfg, featuresBasePath, skipAutoMapping, lockfile, nil) +func fetchFeatureSets(ctx context.Context, logger log.Logger, reg oci.Registry, featuresCfg map[string]interface{}, featuresBasePath string, skipAutoMapping bool, lockfile *features.Lockfile) (*fetchFeatureResult, error) { + return fetchFeatureSetsWithOrder(ctx, logger, reg, featuresCfg, featuresBasePath, skipAutoMapping, lockfile, nil) } // fetchFeatureSetsWithOrder resolves the feature dependency graph through the @@ -89,7 +89,7 @@ func fetchFeatureSets(logger log.Logger, reg oci.Registry, featuresCfg map[strin // order. Each returned Set's content is staged under the returned TmpDir // at _dev_container_feature_, matching the generated // Dockerfile's COPY paths. -func fetchFeatureSetsWithOrder(logger log.Logger, reg oci.Registry, featuresCfg map[string]interface{}, featuresBasePath string, skipAutoMapping bool, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) { +func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, reg oci.Registry, featuresCfg map[string]interface{}, featuresBasePath string, skipAutoMapping bool, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) { if len(featuresCfg) == 0 { return nil, nil } @@ -104,7 +104,7 @@ func fetchFeatureSetsWithOrder(logger log.Logger, reg oci.Registry, featuresCfg ociClient := reg if ociClient == nil { - ociClient = oci.NewClient(logger, osEnvMap()) + ociClient = newOCIClient(ctx, logger) } tmpDir, err := os.MkdirTemp("", "devcontainer-features-") @@ -396,6 +396,7 @@ func processInstallFeature( "owner": strings.SplitN(ref.Namespace, "/", 2)[0], "path": ref.Namespace + "/" + ref.ID, "registry": ref.Registry, + "scheme": ref.Scheme(), "resource": ref.Resource, "tag": ref.Tag, "version": ref.Tag, }, @@ -529,7 +530,7 @@ func extendImageWithFeatures( if fbOpts != nil { overrideOrder = fbOpts.OverrideFeatureInstallOrder } - result, err := fetchFeatureSetsWithOrder(logger, nil, featuresCfg, featuresBasePath, skipAutoMap, lockfile, overrideOrder) + result, err := fetchFeatureSetsWithOrder(ctx, logger, nil, featuresCfg, featuresBasePath, skipAutoMap, lockfile, overrideOrder) if err != nil { return nil, err } diff --git a/internal/cli/feature_install_test.go b/internal/cli/feature_install_test.go index a3bbf0c..c5c720f 100644 --- a/internal/cli/feature_install_test.go +++ b/internal/cli/feature_install_test.go @@ -134,7 +134,7 @@ func TestFetchFeatureSets(t *testing.T) { t.Run(tt.name, func(t *testing.T) { baseDir := t.TempDir() tt.setup(t, baseDir) - result, err := fetchFeatureSets(log.Null, nil, tt.entries, baseDir, false, nil) + result, err := fetchFeatureSets(t.Context(), log.Null, nil, tt.entries, baseDir, false, nil) if result != nil && result.TmpDir != "" { defer os.RemoveAll(result.TmpDir) } diff --git a/internal/cli/features_info.go b/internal/cli/features_info.go index 5e586eb..5637e35 100644 --- a/internal/cli/features_info.go +++ b/internal/cli/features_info.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "context" "encoding/json" "fmt" "os" @@ -29,7 +30,7 @@ func realFeaturesInfoCmd() *cobra.Command { return fmt.Errorf("Invalid mode %q. Choose from: manifest, tags, dependencies, verbose", mode) } featureID := args[1] - return runFeaturesInfo(outputFor(cmd), mode, featureID, logLevel, outputFormat) + return runFeaturesInfo(cmd.Context(), outputFor(cmd), mode, featureID, logLevel, outputFormat) }, } @@ -39,7 +40,7 @@ func realFeaturesInfoCmd() *cobra.Command { return cmd } -func runFeaturesInfo(out Output, mode, featureID, logLevel, outputFormat string) error { +func runFeaturesInfo(ctx context.Context, out Output, mode, featureID, logLevel, outputFormat string) error { for _, v := range []struct { flag, val string choices []string @@ -66,8 +67,7 @@ func runFeaturesInfo(out Output, mode, featureID, logLevel, outputFormat string) return fmt.Errorf("Failed to parse Feature identifier %q", featureID) } - env := osEnvMap() - client := oci.NewClient(logger, env) + client := newOCIClient(ctx, logger) jsonOutput := make(map[string]interface{}) diff --git a/internal/cli/features_resolve_deps.go b/internal/cli/features_resolve_deps.go index b5773c7..b0adb28 100644 --- a/internal/cli/features_resolve_deps.go +++ b/internal/cli/features_resolve_deps.go @@ -10,7 +10,6 @@ import ( coreerrors "github.com/devcontainers/cli/internal/errors" "github.com/devcontainers/cli/internal/features" "github.com/devcontainers/cli/internal/log" - "github.com/devcontainers/cli/internal/oci" "github.com/spf13/cobra" ) @@ -52,7 +51,7 @@ func realFeaturesResolveDepsCmd() *cobra.Command { return &coreerrors.ExitCodeError{Code: 1} } - ociClient := oci.NewClient(logger, osEnvMap()) + ociClient := newOCIClient(cmd.Context(), logger) // Read the lockfile (if any) so tarball/OCI resolution can pin digests. var lockfile *features.Lockfile diff --git a/internal/cli/features_test_runner.go b/internal/cli/features_test_runner.go index 2aac310..ba811f5 100644 --- a/internal/cli/features_test_runner.go +++ b/internal/cli/features_test_runner.go @@ -94,6 +94,9 @@ func runFeaturesTestCommand( preserveContainers bool, quiet bool, permitRandomization bool, + // globalArgs carries the invocation's global flags (e.g. --oci-auth-hardening) + // so the `up` the runner spawns for each test project honors them too. + globalArgs []string, ) int { // Print the banner before validating the collection layout so that a // project folder lacking src/ and test/ still emits the header. This matches @@ -121,7 +124,7 @@ func runFeaturesTestCommand( var results []testResult if globalOnly { - results = runGlobalTests(logger, collectionFolder, filter, baseImage, remoteUser, quiet) + results = runGlobalTests(logger, collectionFolder, filter, baseImage, remoteUser, quiet, globalArgs) } else { // Discover features to test features := featuresList @@ -152,20 +155,20 @@ func runFeaturesTestCommand( // Run autogenerated tests (test.sh for each feature) if !skipAuto { - results = append(results, runAutoTests(logger, collectionFolder, features, baseImage, remoteUser, quiet)...) + results = append(results, runAutoTests(logger, collectionFolder, features, baseImage, remoteUser, quiet, globalArgs)...) } // Run scenario tests if !skipScenarios { for _, feature := range features { featureTestDir := filepath.Join(testDir, feature) - results = append(results, runScenarioTests(logger, collectionFolder, featureTestDir, feature, filter, baseImage, remoteUser, quiet)...) + results = append(results, runScenarioTests(logger, collectionFolder, featureTestDir, feature, filter, baseImage, remoteUser, quiet, globalArgs)...) } } // Run global tests (unless features were explicitly specified) if featuresList == nil { - results = append(results, runGlobalTests(logger, collectionFolder, filter, baseImage, remoteUser, quiet)...) + results = append(results, runGlobalTests(logger, collectionFolder, filter, baseImage, remoteUser, quiet, globalArgs)...) } } @@ -177,7 +180,7 @@ func runFeaturesTestCommand( return reportTestResults(out, results) } -func runAutoTests(logger log.Logger, collectionFolder string, features []string, baseImage, remoteUser string, quiet bool) []testResult { +func runAutoTests(logger log.Logger, collectionFolder string, features []string, baseImage, remoteUser string, quiet bool, globalArgs []string) []testResult { var results []testResult // Create a single container with all features installed @@ -225,7 +228,7 @@ func runAutoTests(logger log.Logger, collectionFolder string, features []string, // Build and start container logger.Write("Building test container...", log.LevelInfo) - containerId, err := upTestContainer(tmpDir, quiet) + containerId, err := upTestContainer(tmpDir, quiet, globalArgs) if err != nil { logger.Write(fmt.Sprintf("Failed to start container: %v", err), log.LevelError) return []testResult{{Name: "autogenerated setup", Status: testError, Detail: err.Error()}} @@ -263,7 +266,7 @@ func runAutoTests(logger log.Logger, collectionFolder string, features []string, return results } -func runScenarioTests(logger log.Logger, collectionFolder, featureTestDir, feature, filter, baseImage, remoteUser string, quiet bool) []testResult { +func runScenarioTests(logger log.Logger, collectionFolder, featureTestDir, feature, filter, baseImage, remoteUser string, quiet bool, globalArgs []string) []testResult { var results []testResult scenariosPath := filepath.Join(featureTestDir, "scenarios.json") @@ -351,7 +354,7 @@ func runScenarioTests(logger log.Logger, collectionFolder, featureTestDir, featu continue } - containerId, err := upTestContainer(tmpDir, quiet) + containerId, err := upTestContainer(tmpDir, quiet, globalArgs) if err != nil { logger.Write(fmt.Sprintf("Failed to start scenario container: %v", err), log.LevelError) results = append(results, testResult{Name: scenarioName, Status: testError, Detail: err.Error()}) @@ -385,25 +388,25 @@ func runScenarioTests(logger log.Logger, collectionFolder, featureTestDir, featu return results } -func runGlobalTests(logger log.Logger, collectionFolder, filter, baseImage, remoteUser string, quiet bool) []testResult { +func runGlobalTests(logger log.Logger, collectionFolder, filter, baseImage, remoteUser string, quiet bool, globalArgs []string) []testResult { globalDir := filepath.Join(collectionFolder, "test", "_global") if !pfs.IsDir(globalDir) { return nil } - return runScenarioTests(logger, collectionFolder, globalDir, "_global", filter, baseImage, remoteUser, quiet) + return runScenarioTests(logger, collectionFolder, globalDir, "_global", filter, baseImage, remoteUser, quiet, globalArgs) } -func upTestContainer(workspaceFolder string, quiet bool) (string, error) { +func upTestContainer(workspaceFolder string, quiet bool, globalArgs []string) (string, error) { // Tag the container so cleanupTestContainers (which filters on this label) // can remove it — otherwise every test run leaks a container. A second, // per-workspace label keeps each autotest/scenario container distinct: `up` // matches an existing container by --id-label, so without a unique label the // scenario up would reuse (and fail to start) the autotest's container. - args := []string{ + args := append(append([]string{}, globalArgs...), []string{ "up", "--workspace-folder", workspaceFolder, "--skip-post-create", "--log-level", "info", "--id-label", "devcontainer.is_test_run=true", "--id-label", "devcontainer.test_id=" + filepath.Base(workspaceFolder), - } + }...) if quiet { args = append(args, "--log-level", "error") } diff --git a/internal/cli/features_test_runner_test.go b/internal/cli/features_test_runner_test.go index a953096..74055a2 100644 --- a/internal/cli/features_test_runner_test.go +++ b/internal/cli/features_test_runner_test.go @@ -14,7 +14,7 @@ func TestRunAutoTests_ReportsStagingError(t *testing.T) { t.Fatal(err) } - results := runAutoTests(log.Null, base, []string{"missing"}, "alpine", "", true) + results := runAutoTests(log.Null, base, []string{"missing"}, "alpine", "", true, nil) if len(results) != 1 || results[0].Status != testError { t.Fatalf("results = %#v, want one setup error", results) } @@ -30,7 +30,7 @@ func TestRunScenarioTests_ReportsInvalidScenarios(t *testing.T) { t.Fatal(err) } - results := runScenarioTests(log.Null, base, testDir, "sample", "", "alpine", "", true) + results := runScenarioTests(log.Null, base, testDir, "sample", "", "alpine", "", true, nil) if len(results) != 1 || results[0].Status != testError { t.Fatalf("results = %#v, want one parse error", results) } @@ -46,7 +46,7 @@ func TestRunScenarioTests_ReportsMissingScriptAsSkipped(t *testing.T) { t.Fatal(err) } - results := runScenarioTests(log.Null, base, testDir, "sample", "", "alpine", "", true) + results := runScenarioTests(log.Null, base, testDir, "sample", "", "alpine", "", true, nil) if len(results) != 1 || results[0].Status != testSkipped { t.Fatalf("results = %#v, want one skipped scenario", results) } diff --git a/internal/cli/flag_inventory_test.go b/internal/cli/flag_inventory_test.go index af66277..3151882 100644 --- a/internal/cli/flag_inventory_test.go +++ b/internal/cli/flag_inventory_test.go @@ -133,6 +133,31 @@ func TestFlagInventoryParity(t *testing.T) { } } + // Global flags live on the root's persistent flag set and are mirrored by the + // YAML `global_flags` block, diffed in both directions like the per-command ones. + actualGlobals := map[string]reflectedFlag{} + root.PersistentFlags().VisitAll(func(f *pflag.Flag) { + actualGlobals[f.Name] = reflectedFlag{ + shorthand: f.Shorthand, + typ: f.Value.Type(), + defValue: f.DefValue, + hidden: f.Hidden, + } + }) + for _, name := range sortedFlagNames(actualGlobals) { + wf, ok := inv.GlobalFlags[name] + if !ok { + report("global flag --%s is declared on the root command but missing from the YAML", name) + continue + } + compareFlag("", name, wf, actualGlobals[name], report) + } + for _, name := range sortedYAMLFlagNames(inv.GlobalFlags) { + if _, ok := actualGlobals[name]; !ok { + report("global flag --%s is in the YAML but not declared on the root command", name) + } + } + if len(problems) > 0 { sort.Strings(problems) t.Fatalf("flag inventory drift (%d):\n%s", len(problems), strings.Join(problems, "\n")) @@ -164,7 +189,8 @@ type yamlCommand struct { } type yamlInventory struct { - Commands map[string]yamlCommand `yaml:"commands"` + Commands map[string]yamlCommand `yaml:"commands"` + GlobalFlags map[string]yamlFlag `yaml:"global_flags"` } // flattenYAMLCommand walks the YAML command/subcommand tree into a flat diff --git a/internal/cli/ociauth.go b/internal/cli/ociauth.go new file mode 100644 index 0000000..e7c21be --- /dev/null +++ b/internal/cli/ociauth.go @@ -0,0 +1,97 @@ +package cli + +import ( + "context" + "fmt" + + "github.com/devcontainers/cli/internal/log" + "github.com/devcontainers/cli/internal/oci" + "github.com/spf13/cobra" +) + +// Global OCI authentication flags, declared on the root command so every +// subcommand accepts them (yargs `.option(..., { global: true })`). +const ( + flagOCIAuthHardening = "oci-auth-hardening" + flagAllowCrossOriginAuthHos = "allow-cross-origin-auth-host" +) + +// ociAuthContextKey keys the per-invocation OCI auth policy in the command context. +type ociAuthContextKey struct{} + +// addOCIAuthFlags declares the global OCI auth flags on the root command. +func addOCIAuthFlags(root *cobra.Command) { + f := root.PersistentFlags() + f.Bool(flagOCIAuthHardening, false, "Restrict OCI bearer authentication realms, registry credential forwarding, and token redirects.") + f.StringArray(flagAllowCrossOriginAuthHos, nil, "Allow an OCI registry to use a cross-origin HTTPS authentication host. Format: =. May be repeated.") +} + +// buildOCIAuthPolicy validates the global OCI auth flags and builds the policy +// for this invocation. It mirrors the reference CLI's yargs `.check()`: +// --allow-cross-origin-auth-host requires --oci-auth-hardening, and every entry +// must be a '=' pair of bare authorities. +func buildOCIAuthPolicy(cmd *cobra.Command, logger log.Logger) (*oci.AuthPolicy, error) { + hardening, _ := cmd.Flags().GetBool(flagOCIAuthHardening) + hosts, _ := cmd.Flags().GetStringArray(flagAllowCrossOriginAuthHos) + if len(hosts) > 0 && !hardening { + return nil, fmt.Errorf("--allow-cross-origin-auth-host requires --oci-auth-hardening.") + } + return oci.NewAuthPolicy(hardening, hosts, logger) +} + +// applyOCIAuthPolicy validates the global flags and stores the resulting policy on +// the command's context, where the command's OCI clients pick it up. +func applyOCIAuthPolicy(cmd *cobra.Command) error { + policy, err := buildOCIAuthPolicy(cmd, log.Null) + if err != nil { + return err + } + cmd.SetContext(withOCIAuthPolicy(cmd.Context(), policy)) + return nil +} + +func withOCIAuthPolicy(ctx context.Context, policy *oci.AuthPolicy) context.Context { + if ctx == nil { + ctx = context.Background() + } + return context.WithValue(ctx, ociAuthContextKey{}, policy) +} + +// ociAuthPolicy returns the policy stored on ctx, or a default (hardening off) +// policy when a caller runs outside the command tree, e.g. a unit test. +func ociAuthPolicy(ctx context.Context) *oci.AuthPolicy { + if ctx != nil { + if policy, ok := ctx.Value(ociAuthContextKey{}).(*oci.AuthPolicy); ok && policy != nil { + return policy + } + } + return oci.DefaultAuthPolicy() +} + +// newOCIClient builds an OCI client bound to this invocation's auth policy, so +// every registry request of the command honors the hardening flags and feeds the +// same diagnostics. +func newOCIClient(ctx context.Context, logger log.Logger) *oci.Client { + return oci.NewClientWithAuthPolicy(logger, osEnvMap(), ociAuthPolicy(ctx)) +} + +// ociAuthDiagnostics returns the diagnostics recorded for this invocation, for +// the `ociAuthDiagnostics` field of the command's JSON output. +func ociAuthDiagnostics(ctx context.Context) oci.AuthDiagnostics { + return ociAuthPolicy(ctx).Diagnostics() +} + +// ociAuthGlobalArgs re-renders the global OCI auth flags as CLI arguments, so a +// command that re-invokes this binary (e.g. `features test` spawning `up` for each +// test project) passes the same policy on to the child process. +func ociAuthGlobalArgs(cmd *cobra.Command) []string { + var args []string + if hardening, _ := cmd.Flags().GetBool(flagOCIAuthHardening); hardening { + args = append(args, "--"+flagOCIAuthHardening) + } + hosts, _ := cmd.Flags().GetStringArray(flagAllowCrossOriginAuthHos) + for _, h := range hosts { + args = append(args, "--"+flagAllowCrossOriginAuthHos, h) + } + return args +} diff --git a/internal/cli/ociauth_test.go b/internal/cli/ociauth_test.go new file mode 100644 index 0000000..b24aa60 --- /dev/null +++ b/internal/cli/ociauth_test.go @@ -0,0 +1,242 @@ +package cli + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devcontainers/cli/internal/oci" + "github.com/spf13/cobra" +) + +// TestOCIAuthGlobalFlagValidation pins the yargs `.check()` behavior the reference +// CLI applies to the global OCI auth flags: --allow-cross-origin-auth-host needs +// --oci-auth-hardening, entries must be '=' pairs of +// bare authorities, and the error message is the one users see (exit code 1). +func TestOCIAuthGlobalFlagValidation(t *testing.T) { + tests := []struct { + name string + args []string + want string // "" means the flags are accepted + }{ + { + name: "allow list requires hardening", + args: []string{"--allow-cross-origin-auth-host", "registry.example=auth.example"}, + want: "--allow-cross-origin-auth-host requires --oci-auth-hardening.", + }, + { + name: "entry must be a pair", + args: []string{"--oci-auth-hardening", "--allow-cross-origin-auth-host", "bad"}, + want: "Invalid cross-origin auth host 'bad'. Expected '='.", + }, + { + name: "entry must hold exactly one separator", + args: []string{"--oci-auth-hardening", "--allow-cross-origin-auth-host", "a=b=c"}, + want: "Invalid cross-origin auth host 'a=b=c'. Expected '='.", + }, + { + name: "entry must be a bare authority", + args: []string{"--oci-auth-hardening", "--allow-cross-origin-auth-host", "a/b=c"}, + want: "Invalid authority 'a/b'.", + }, + { + name: "valid mapping is accepted", + args: []string{"--oci-auth-hardening", "--allow-cross-origin-auth-host", "registry.example=auth.example"}, + }, + { + name: "hardening alone is accepted", + args: []string{"--oci-auth-hardening"}, + }, + { + name: "repeated mappings are accepted", + args: []string{"--oci-auth-hardening", + "--allow-cross-origin-auth-host", "registry.example=auth.example", + "--allow-cross-origin-auth-host", "registry.example=other.example"}, + }, + } + + ws := t.TempDir() + if err := os.MkdirAll(filepath.Join(ws, ".devcontainer"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(ws, ".devcontainer", "devcontainer.json"), []byte(`{"image":"ubuntu"}`), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := NewRootCommand() + args := append(append([]string{}, tt.args...), "read-configuration", "--workspace-folder", ws) + root.SetArgs(args) + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + err := root.Execute() + if tt.want == "" { + if err != nil { + t.Fatalf("flags should be accepted, got error: %v", err) + } + return + } + if err == nil { + t.Fatalf("expected error %q, command succeeded", tt.want) + } + if err.Error() != tt.want { + t.Errorf("error = %q, want %q", err.Error(), tt.want) + } + }) + } +} + +// TestExecAcceptsGlobalOCIAuthFlags guards the `exec` path: it parses its own +// flags (DisableFlagParsing), so it must both accept the global flags and apply +// the same validation, without swallowing the command that follows them. +func TestExecAcceptsGlobalOCIAuthFlags(t *testing.T) { + flags, cmd := splitExecArgs([]string{ + "--oci-auth-hardening", + "--allow-cross-origin-auth-host", "registry.example=auth.example", + "--workspace-folder", "/ws", + "echo", "hello", + }) + wantFlags := []string{"--oci-auth-hardening", "--allow-cross-origin-auth-host", "registry.example=auth.example", "--workspace-folder", "/ws"} + if strings.Join(flags, " ") != strings.Join(wantFlags, " ") { + t.Errorf("flags = %v, want %v", flags, wantFlags) + } + if strings.Join(cmd, " ") != "echo hello" { + t.Errorf("command = %v, want [echo hello]", cmd) + } + + // The check runs for exec too, even though the root hook is skipped there. + root := NewRootCommand() + root.SetArgs([]string{"exec", "--allow-cross-origin-auth-host", "registry.example=auth.example", "echo", "hi"}) + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + err := root.Execute() + if err == nil || err.Error() != "--allow-cross-origin-auth-host requires --oci-auth-hardening." { + t.Errorf("exec error = %v, want the hardening requirement error", err) + } +} + +// TestReadConfigurationEmitsOCIAuthDiagnostics pins the new output field: the +// reference CLI emits ociAuthDiagnostics on every read-configuration, even when +// no registry was contacted. +func TestReadConfigurationEmitsOCIAuthDiagnostics(t *testing.T) { + ws := t.TempDir() + if err := os.MkdirAll(filepath.Join(ws, ".devcontainer"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(ws, ".devcontainer", "devcontainer.json"), []byte(`{"image":"ubuntu"}`), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + var stdout bytes.Buffer + root := NewRootCommand() + root.SetArgs([]string{"read-configuration", "--workspace-folder", ws}) + root.SetOut(&stdout) + root.SetErr(&bytes.Buffer{}) + if err := root.Execute(); err != nil { + t.Fatalf("read-configuration: %v", err) + } + + var parsed struct { + OCIAuthDiagnostics *oci.AuthDiagnostics `json:"ociAuthDiagnostics"` + } + if err := json.Unmarshal(stdout.Bytes(), &parsed); err != nil { + t.Fatalf("parse output %q: %v", stdout.String(), err) + } + if parsed.OCIAuthDiagnostics == nil { + t.Fatalf("ociAuthDiagnostics missing from output: %s", stdout.String()) + } + if *parsed.OCIAuthDiagnostics != (oci.AuthDiagnostics{}) { + t.Errorf("no registry was contacted, want all-false diagnostics, got %+v", *parsed.OCIAuthDiagnostics) + } +} + +// TestOCIAuthPolicyFlowsFromCommandContext guards the plumbing: the policy built +// from the global flags must reach the OCI clients a command creates. +func TestOCIAuthPolicyFlowsFromCommandContext(t *testing.T) { + probe := &cobra.Command{ + Use: "probe", + RunE: func(c *cobra.Command, _ []string) error { + if !ociAuthPolicy(c.Context()).Hardening() { + t.Error("--oci-auth-hardening did not reach the command context") + } + return nil + }, + } + root := NewRootCommand() + root.AddCommand(probe) + root.SetArgs([]string{"--oci-auth-hardening", "probe"}) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + if err := root.Execute(); err != nil { + t.Fatalf("probe: %v", err) + } + + // Without the flag the default policy applies (hardening off). + plain := NewRootCommand() + plain.AddCommand(&cobra.Command{ + Use: "probe", + RunE: func(c *cobra.Command, _ []string) error { + if ociAuthPolicy(c.Context()).Hardening() { + t.Error("hardening must default to off") + } + return nil + }, + }) + plain.SetArgs([]string{"probe"}) + plain.SetOut(&bytes.Buffer{}) + plain.SetErr(&bytes.Buffer{}) + if err := plain.Execute(); err != nil { + t.Fatalf("probe: %v", err) + } +} + +// TestOCIAuthGlobalArgsForwarding pins the re-invocation path: `features test` +// spawns this binary for each test project's `up`, so the global auth flags must +// be rendered back into arguments (upstream passes them in-process instead). +func TestOCIAuthGlobalArgsForwarding(t *testing.T) { + tests := []struct { + name string + args []string + want []string + }{ + {"no flags", []string{"probe"}, nil}, + {"hardening only", []string{"--oci-auth-hardening", "probe"}, []string{"--oci-auth-hardening"}}, + { + "hardening with repeated mappings", + []string{"--oci-auth-hardening", + "--allow-cross-origin-auth-host", "registry.example=auth.example", + "--allow-cross-origin-auth-host", "other.example=auth.other", "probe"}, + []string{"--oci-auth-hardening", + "--allow-cross-origin-auth-host", "registry.example=auth.example", + "--allow-cross-origin-auth-host", "other.example=auth.other"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got []string + root := NewRootCommand() + root.AddCommand(&cobra.Command{ + Use: "probe", + RunE: func(c *cobra.Command, _ []string) error { + got = ociAuthGlobalArgs(c) + return nil + }, + }) + root.SetArgs(tt.args) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + if err := root.Execute(); err != nil { + t.Fatalf("probe: %v", err) + } + if strings.Join(got, " ") != strings.Join(tt.want, " ") { + t.Errorf("args = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/cli/oracle_flag_coverage_test.go b/internal/cli/oracle_flag_coverage_test.go index 9148707..7ece658 100644 --- a/internal/cli/oracle_flag_coverage_test.go +++ b/internal/cli/oracle_flag_coverage_test.go @@ -35,6 +35,15 @@ func TestOracleFlagCoverage(t *testing.T) { inventory := inventoryCommandFlags(t) var problems []string + + // Global options (yargs `.option(..., { global: true })`) belong to every + // command; the inventory tracks them under `global_flags`. + inventoryGlobals := inventoryGlobalFlags(t) + for f := range oracleGlobalFlags(string(src)) { + if !inventoryGlobals[f] { + problems = append(problems, " --"+f) + } + } for cmd, flags := range oracle { invFlags, ok := inventory[cmd] if !ok { @@ -55,6 +64,44 @@ func TestOracleFlagCoverage(t *testing.T) { } } +// oracleGlobalFlags returns the flag names the oracle declares as global options +// on the root yargs chain (before `.strict()`), i.e. the ones every command +// accepts. +func oracleGlobalFlags(src string) map[string]bool { + start := strings.Index(src, ".scriptName('devcontainer')") + end := strings.Index(src, ".strict();") + if start < 0 || end < 0 || end <= start { + return nil + } + chain := src[start:end] + optionRe := regexp.MustCompile(`\.option\('([a-z][a-z0-9-]*)',\s*\{`) + out := map[string]bool{} + for _, m := range optionRe.FindAllStringSubmatch(chain, -1) { + out[m[1]] = true + } + return out +} + +// inventoryGlobalFlags loads the global flag names declared in the YAML. +func inventoryGlobalFlags(t *testing.T) map[string]bool { + t.Helper() + data, err := os.ReadFile("../../docs/parity/cli-flags-inventory.yaml") + if err != nil { + t.Fatalf("read inventory: %v", err) + } + var doc struct { + GlobalFlags map[string]yaml.Node `yaml:"global_flags"` + } + if err := yaml.Unmarshal(data, &doc); err != nil { + t.Fatalf("parse inventory: %v", err) + } + out := map[string]bool{} + for name := range doc.GlobalFlags { + out[name] = true + } + return out +} + // oracleCommandFlags maps each top-level command name to the set of flag names // its options function declares in the oracle source. func oracleCommandFlags(src string) map[string]map[string]bool { diff --git a/internal/cli/outdated.go b/internal/cli/outdated.go index 99b76a6..caffdae 100644 --- a/internal/cli/outdated.go +++ b/internal/cli/outdated.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "context" "encoding/json" "fmt" "os" @@ -66,7 +67,7 @@ func newOutdatedCmd() *cobra.Command { if opts.workspaceFolder == "" { opts.workspaceFolder, _ = os.Getwd() } - return runOutdated(outputFor(cmd), opts) + return runOutdated(cmd.Context(), outputFor(cmd), opts) }, } @@ -158,7 +159,7 @@ func majorOf(v string) string { return fmt.Sprintf("%d", parsed.Major()) } -func runOutdated(out Output, opts outdatedOpts) error { +func runOutdated(ctx context.Context, out Output, opts outdatedOpts) error { logDst, closeLog, logErr := logWriter(opts.logFile, opts.terminalLogFile) if logErr != nil { return fmt.Errorf("open log file: %w", logErr) @@ -192,7 +193,7 @@ func runOutdated(out Output, opts outdatedOpts) error { return nil } - ociClient := oci.NewClient(logger, osEnvMap()) + ociClient := newOCIClient(ctx, logger) // Lockfile pins the concrete "current" version when present (matches TS // loadVersionInfo: current = lockfileVersion || wanted). @@ -459,7 +460,7 @@ func newUpgradeCmd() *cobra.Command { // Generate new lockfile from current features config // This is a simplified version — the full implementation // would resolve all features via OCI and compute digests. - ociClient := oci.NewClient(logger, osEnvMap()) + ociClient := newOCIClient(cmd.Context(), logger) featureSets := resolveFeatureSets(cfg, ociClient, logger) lf := features.GenerateLockfile(&features.Config{FeatureSets: featureSets}, nil) diff --git a/internal/cli/read_configuration.go b/internal/cli/read_configuration.go index 9a8d52a..999b8a4 100644 --- a/internal/cli/read_configuration.go +++ b/internal/cli/read_configuration.go @@ -274,7 +274,7 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) needsFeaturesConfig := opts.includeFeaturesCfg || (opts.includeMergedCfg && containerID == "") if needsFeaturesConfig && result != nil && len(result.Config.Features) > 0 { lgr := log.New(log.Options{Level: log.ParseLevel(opts.logLevel), Format: opts.logFormat, Writer: logDst, Dimensions: logDimensions(opts.terminalColumns, opts.terminalRows)}) - featResult, featErr := fetchFeatureSets(lgr, nil, result.Config.Features, filepath.Dir(result.Config.ConfigFilePath), opts.skipFeatureAutoMapping, nil) + featResult, featErr := fetchFeatureSets(ctx, lgr, nil, result.Config.Features, filepath.Dir(result.Config.ConfigFilePath), opts.skipFeatureAutoMapping, nil) if featErr == nil && featResult != nil { defer os.RemoveAll(featResult.TmpDir) output["featuresConfiguration"] = map[string]interface{}{ @@ -311,7 +311,7 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) } } if len(result.Config.Features) > 0 { - if fr, ferr := fetchFeatureSets(lgr, nil, result.Config.Features, filepath.Dir(result.Config.ConfigFilePath), opts.skipFeatureAutoMapping, nil); ferr == nil && fr != nil { + if fr, ferr := fetchFeatureSets(ctx, lgr, nil, result.Config.Features, filepath.Dir(result.Config.ConfigFilePath), opts.skipFeatureAutoMapping, nil); ferr == nil && fr != nil { defer os.RemoveAll(fr.TmpDir) for _, fs := range fr.FeatureSets { entries = append(entries, featureMetadataEntry(fs, false)) @@ -372,6 +372,10 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) } } + // OCI auth diagnostics for this invocation (TS readConfiguration emits it + // unconditionally, even when no registry was contacted). + output["ociAuthDiagnostics"] = ociAuthDiagnostics(ctx) + data, err := json.Marshal(output) if err != nil { return fmt.Errorf("marshal output: %w", err) diff --git a/internal/cli/root.go b/internal/cli/root.go index 0be1ed3..f9bf8b1 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -28,12 +28,24 @@ func NewRootCommand() *cobra.Command { // Match yargs: boolean-negation disabled, strict mode SilenceUsage: true, SilenceErrors: true, + // Validate the global OCI auth flags before any subcommand runs and stash + // the resulting policy on the context (yargs `.check()` equivalent). + // `exec` parses its own flags (DisableFlagParsing), so it applies the + // policy itself once its flags are parsed. + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + if cmd.DisableFlagParsing { + return nil + } + return applyOCIAuthPolicy(cmd) + }, } // Print just the bare version (e.g. "0.74.0"), matching the TS CLI (yargs // .version()) instead of Cobra's " version " template. root.SetVersionTemplate("{{.Version}}\n") + addOCIAuthFlags(root) + // Register subcommands root.AddCommand( newReadConfigurationCmd(), diff --git a/internal/cli/templates_apply.go b/internal/cli/templates_apply.go index 584180f..9f7c2c3 100644 --- a/internal/cli/templates_apply.go +++ b/internal/cli/templates_apply.go @@ -7,7 +7,6 @@ import ( "github.com/devcontainers/cli/internal/jsonc" "github.com/devcontainers/cli/internal/log" - "github.com/devcontainers/cli/internal/oci" "github.com/devcontainers/cli/internal/templates" "github.com/spf13/cobra" ) @@ -66,7 +65,7 @@ func realTemplatesApplyCmd() *cobra.Command { } } - ociClient := oci.NewClient(logger, osEnvMap()) + ociClient := newOCIClient(cmd.Context(), logger) selected := templates.SelectedTemplate{ ID: templateID, diff --git a/internal/cli/templates_metadata.go b/internal/cli/templates_metadata.go index 70829e9..4bf9c8e 100644 --- a/internal/cli/templates_metadata.go +++ b/internal/cli/templates_metadata.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "context" "encoding/json" "fmt" "os" @@ -19,7 +20,7 @@ func realTemplatesMetadataCmd() *cobra.Command { Short: "Fetch a published Template's metadata", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runTemplatesMetadata(outputFor(cmd), args[0], logLevel) + return runTemplatesMetadata(cmd.Context(), outputFor(cmd), args[0], logLevel) }, } @@ -27,7 +28,7 @@ func realTemplatesMetadataCmd() *cobra.Command { return cmd } -func runTemplatesMetadata(out Output, templateID, logLevel string) error { +func runTemplatesMetadata(ctx context.Context, out Output, templateID, logLevel string) error { logger := log.New(log.Options{ Level: log.ParseLevel(logLevel), Format: "text", @@ -40,7 +41,7 @@ func runTemplatesMetadata(out Output, templateID, logLevel string) error { return fmt.Errorf("parse template identifier %q: %w", templateID, err) } - client := oci.NewClient(logger, osEnvMap()) + client := newOCIClient(ctx, logger) manifest, err := client.FetchManifest(ref, "") if err != nil { diff --git a/internal/cli/up.go b/internal/cli/up.go index 7295c8a..3f90006 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -257,6 +257,8 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error { Dimensions: logDimensions(opts.terminalColumns, opts.terminalRows), Secrets: secretValuesFromFile(opts.secretsFile), }) + // Route the OCI auth diagnostic lines at this command's logger. + ociAuthPolicy(ctx).SetLogger(logger) // Engine SDK client for container/image operations engine, err := docker.NewEngineClient(logger) @@ -493,6 +495,7 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error { "containerId": containerID, "remoteUser": remoteUser, "remoteWorkspaceFolder": remoteWorkspaceFolder, + "ociAuthDiagnostics": ociAuthDiagnostics(ctx), } if opts.composeProjectName != "" { result["composeProjectName"] = opts.composeProjectName @@ -647,6 +650,7 @@ func (r *upRunner) finishUp(ctx context.Context, containerID string, cfg *config "containerId": containerID, "remoteUser": remoteUser, "remoteWorkspaceFolder": remoteWorkspaceFolder, + "ociAuthDiagnostics": ociAuthDiagnostics(ctx), } return writeSuccessJSON(out, result) @@ -1269,7 +1273,7 @@ func (r *upRunner) fromCompose(ctx context.Context, cfg *config.DevContainer, lo serviceBuildTarget := svcBuild.Target // Fetch and resolve features - fetchResult, fetchErr := fetchFeatureSets(logger, nil, cfg.Features, filepath.Dir(cfg.ConfigFilePath), opts.skipFeatureAutoMapping, nil) + fetchResult, fetchErr := fetchFeatureSets(ctx, logger, nil, cfg.Features, filepath.Dir(cfg.ConfigFilePath), opts.skipFeatureAutoMapping, nil) if fetchErr != nil { return "", fetchErr } diff --git a/internal/imagemeta/extend.go b/internal/imagemeta/extend.go index ae95a64..32cc038 100644 --- a/internal/imagemeta/extend.go +++ b/internal/imagemeta/extend.go @@ -39,7 +39,7 @@ func GenerateExtendImageBuild( if len(featureSets) == 0 { // No features — just add metadata label df := docker.NewDockerfileBuilder() - df.Arg("_DEV_CONTAINERS_BASE_IMAGE", "placeholder") + df.Arg("_DEV_CONTAINERS_BASE_IMAGE", "scratch") prefix := df.String() df2 := docker.NewDockerfileBuilder() @@ -59,7 +59,7 @@ func GenerateExtendImageBuild( df := docker.NewDockerfileBuilder() // Preamble - df.Arg("_DEV_CONTAINERS_BASE_IMAGE", "placeholder") + df.Arg("_DEV_CONTAINERS_BASE_IMAGE", "scratch") if useBuildKitContexts { df.From("scratch").As("dev_containers_feature_content_source") diff --git a/internal/oci/authpolicy.go b/internal/oci/authpolicy.go new file mode 100644 index 0000000..cb898cc --- /dev/null +++ b/internal/oci/authpolicy.go @@ -0,0 +1,360 @@ +package oci + +import ( + "fmt" + "net/http" + "net/url" + "strings" + "sync" + + "github.com/devcontainers/cli/internal/log" +) + +// AuthDiagnostics mirrors the TS OCIAuthDiagnostics record (spec-common/ociAuth.ts). +// It reports what WOULD change if `--oci-auth-hardening` were enabled, so a user +// can measure the compatibility impact before turning the hardening on. The JSON +// field names are part of the `up`/`build`/`read-configuration` output contract. +type AuthDiagnostics struct { + AuthLookupWouldBeBlocked bool `json:"authLookupWouldBeBlocked"` + RegistryRedirectWouldPreventCredentialForwarding bool `json:"registryRedirectWouldPreventCredentialForwarding"` + AuthServerRedirect bool `json:"authServerRedirect"` +} + +// builtInCrossOriginAuthHosts are the registry→auth-host mappings the reference +// CLI trusts out of the box (httpOCIRegistry.ts). +var builtInCrossOriginAuthHosts = []string{ + "registry-1.docker.io=auth.docker.io", + "registry.docker.io=auth.docker.io", + "docker.io=auth.docker.io", + "index.docker.io=auth.docker.io", + "registry.gitlab.com=gitlab.com", +} + +// dockerHubRegistryHosts are the equivalent authorities Docker Hub references and +// distribution requests use interchangeably. +var dockerHubRegistryHosts = map[string]bool{ + "registry-1.docker.io": true, + "registry.docker.io": true, + "docker.io": true, + "index.docker.io": true, +} + +// AuthPolicy carries the OCI authentication policy for one CLI invocation: the +// `--oci-auth-hardening` switch, the trusted cross-origin auth hosts, and the +// diagnostics accumulated while talking to registries. A single value is shared +// by every oci.Client a command builds, so the diagnostics reported in the +// command's JSON output cover all registry traffic of that command. +type AuthPolicy struct { + hardening bool + crossOrigin map[string]map[string]bool + logger log.Logger + + mu sync.Mutex + diag AuthDiagnostics +} + +// NewAuthPolicy builds the policy from the CLI flags. allowedCrossOriginAuthHosts +// entries are '=' pairs, validated the same way the +// reference CLI validates them. +func NewAuthPolicy(hardening bool, allowedCrossOriginAuthHosts []string, logger log.Logger) (*AuthPolicy, error) { + hosts, err := ParseCrossOriginAuthHosts(append(append([]string{}, builtInCrossOriginAuthHosts...), allowedCrossOriginAuthHosts...)) + if err != nil { + return nil, err + } + if logger == nil { + logger = log.Null + } + return &AuthPolicy{hardening: hardening, crossOrigin: hosts, logger: logger}, nil +} + +// DefaultAuthPolicy is the policy used when no flags were parsed (hardening off, +// only the built-in cross-origin mappings). Its diagnostics are discarded. +func DefaultAuthPolicy() *AuthPolicy { + p, err := NewAuthPolicy(false, nil, log.Null) + if err != nil { + // The built-in entries are constants and always parse. + panic(fmt.Sprintf("default OCI auth policy: %v", err)) + } + return p +} + +// SetLogger directs the diagnostic log lines at the command's logger. It is set +// once the command has built its logger, after flag parsing. +func (p *AuthPolicy) SetLogger(logger log.Logger) { + if p == nil || logger == nil { + return + } + p.mu.Lock() + defer p.mu.Unlock() + p.logger = logger +} + +// Hardening reports whether `--oci-auth-hardening` was requested. +func (p *AuthPolicy) Hardening() bool { return p != nil && p.hardening } + +// Diagnostics returns a snapshot of the diagnostics recorded so far. +func (p *AuthPolicy) Diagnostics() AuthDiagnostics { + if p == nil { + return AuthDiagnostics{} + } + p.mu.Lock() + defer p.mu.Unlock() + return p.diag +} + +// record flips a diagnostic once and logs the reason the first time, matching the +// reference CLI's `[httpOci] OCI auth diagnostics: ...` line. +func (p *AuthPolicy) record(field *bool, message string) { + p.mu.Lock() + already := *field + if !already { + *field = true + } + logger := p.logger + p.mu.Unlock() + if !already { + logger.Write("[httpOci] OCI auth diagnostics: "+message, log.LevelInfo) + } +} + +// ParseCrossOriginAuthHosts parses '=' entries into a +// registry → auth-hosts map, rejecting anything that is not a bare authority. +func ParseCrossOriginAuthHosts(entries []string) (map[string]map[string]bool, error) { + out := map[string]map[string]bool{} + for _, entry := range entries { + sep := strings.Index(entry, "=") + if sep <= 0 || sep != strings.LastIndex(entry, "=") || sep == len(entry)-1 { + return nil, fmt.Errorf("Invalid cross-origin auth host '%s'. Expected '='.", entry) + } + registry, err := normalizeHTTPSAuthority(entry[:sep]) + if err != nil { + return nil, err + } + authHost, err := normalizeHTTPSAuthority(entry[sep+1:]) + if err != nil { + return nil, err + } + if out[registry] == nil { + out[registry] = map[string]bool{} + } + out[registry][authHost] = true + } + return out, nil +} + +// normalizeHTTPSAuthority validates that authority is a bare host[:port] and +// returns it lower-cased. +func normalizeHTTPSAuthority(authority string) (string, error) { + invalid := fmt.Errorf("Invalid authority '%s'.", authority) + parsed, err := url.Parse("https://" + authority) + if err != nil || parsed.Host == "" { + return "", invalid + } + if parsed.User != nil || (parsed.Path != "" && parsed.Path != "/") || parsed.RawQuery != "" || parsed.Fragment != "" { + return "", invalid + } + if parsed.Host != authority && !strings.EqualFold(parsed.Host, authority) { + return "", invalid + } + return strings.ToLower(parsed.Host), nil +} + +// isRegistryOrigin reports whether u addresses the registry of ociRef, treating +// the interchangeable Docker Hub authorities as one origin. +func isRegistryOrigin(u, registry *url.URL) bool { + if sameOrigin(u, registry) { + return true + } + return strings.EqualFold(u.Scheme, "https") && strings.EqualFold(registry.Scheme, "https") && + dockerHubRegistryHosts[strings.ToLower(u.Host)] && dockerHubRegistryHosts[strings.ToLower(registry.Host)] +} + +func sameOrigin(a, b *url.URL) bool { + return strings.EqualFold(a.Scheme, b.Scheme) && strings.EqualFold(canonicalAuthority(a), canonicalAuthority(b)) +} + +// canonicalAuthority is host[:port] with the scheme's default port applied. +func canonicalAuthority(u *url.URL) string { + host := strings.ToLower(u.Hostname()) + port := u.Port() + if port == "" { + switch strings.ToLower(u.Scheme) { + case "https": + port = "443" + case "http": + port = "80" + } + } + return host + ":" + port +} + +// allowsTokenServiceRealm reports whether the registry may direct a token request +// to realm: the same authority over HTTPS (or HTTP on localhost), or an HTTPS auth +// host explicitly mapped to that registry. +func (p *AuthPolicy) allowsTokenServiceRealm(realm, registry *url.URL) bool { + if isAllowedSameAuthorityRealm(realm, registry) { + return true + } + if !strings.EqualFold(realm.Scheme, "https") { + return false + } + return p.crossOrigin[strings.ToLower(registry.Host)][strings.ToLower(realm.Host)] +} + +func isAllowedSameAuthorityRealm(realm, registry *url.URL) bool { + if !strings.EqualFold(realm.Host, registry.Host) { + return false + } + return strings.EqualFold(realm.Scheme, "https") || + (strings.EqualFold(realm.Scheme, "http") && strings.EqualFold(realm.Hostname(), "localhost")) +} + +// authPolicyTransport applies the OCI auth policy to one repository's traffic. +// +// oras-go already refuses to forward registry credentials to a challenge that +// arrives from another origin, so this layer adds what the reference CLI's +// hardening adds on top: bearer realms are pinned to the registry authority (or +// an explicitly trusted auth host), token endpoints may not redirect, and the +// three compatibility diagnostics are recorded whether or not hardening is on. +type authPolicyTransport struct { + base http.RoundTripper + policy *AuthPolicy + registry *url.URL + + mu sync.Mutex + // tokenEndpoints holds the scheme://host/path of every realm a challenge + // pointed at, so a token request can be told apart from a registry request. + tokenEndpoints map[string]bool +} + +func newAuthPolicyTransport(base http.RoundTripper, policy *AuthPolicy, registryScheme, registryHost string) *authPolicyTransport { + return &authPolicyTransport{ + base: base, + policy: policy, + registry: &url.URL{Scheme: registryScheme, Host: registryHost}, + tokenEndpoints: map[string]bool{}, + } +} + +func endpointKey(u *url.URL) string { + return strings.ToLower(u.Scheme+"://"+u.Host) + u.Path +} + +func (t *authPolicyTransport) isTokenEndpoint(u *url.URL) bool { + t.mu.Lock() + defer t.mu.Unlock() + return t.tokenEndpoints[endpointKey(u)] +} + +func (t *authPolicyTransport) addTokenEndpoint(u *url.URL) { + t.mu.Lock() + defer t.mu.Unlock() + t.tokenEndpoints[endpointKey(u)] = true +} + +func (t *authPolicyTransport) RoundTrip(req *http.Request) (*http.Response, error) { + tokenRequest := t.isTokenEndpoint(req.URL) + + resp, err := t.base.RoundTrip(req) + if err != nil || resp == nil { + return resp, err + } + + if tokenRequest { + return t.inspectTokenResponse(req, resp) + } + return t.inspectRegistryResponse(req, resp) +} + +// inspectTokenResponse enforces the "token endpoints must not redirect around +// their validated authority boundary" rule and records the redirect diagnostic. +func (t *authPolicyTransport) inspectTokenResponse(req *http.Request, resp *http.Response) (*http.Response, error) { + if resp.StatusCode < 300 || resp.StatusCode > 399 { + return resp, nil + } + location, locErr := resp.Location() + if locErr != nil { + return resp, nil + } + if t.policy.Hardening() { + resp.Body.Close() + return nil, fmt.Errorf("failed to request bearer token for %q: authentication server redirected the token request to %q", req.URL.Host, location.Host) + } + where := fmt.Sprintf("from origin '%s' to '%s'", originOf(req.URL), originOf(location)) + if sameOrigin(req.URL, location) { + where = fmt.Sprintf("within origin '%s'", originOf(req.URL)) + } + t.policy.record(&t.policy.diag.AuthServerRedirect, "Authentication server redirected a token request "+where+".") + // The redirect target serves the same token request; keep treating it as one. + t.addTokenEndpoint(location) + return resp, nil +} + +// inspectRegistryResponse records the credential-forwarding diagnostic and +// validates the bearer realm of an authentication challenge. +func (t *authPolicyTransport) inspectRegistryResponse(req *http.Request, resp *http.Response) (*http.Response, error) { + if resp.StatusCode != http.StatusUnauthorized && resp.StatusCode != http.StatusForbidden { + return resp, nil + } + + if !isRegistryOrigin(req.URL, t.registry) { + t.policy.record(&t.policy.diag.RegistryRedirectWouldPreventCredentialForwarding, + fmt.Sprintf("Request to '%s' with authentication challenge from '%s' would prevent forwarding registry '%s' credentials with OCI auth hardening.", + req.URL.Host, req.URL.Host, t.registry.Host)) + } + + realm := bearerRealm(resp.Header.Get("WWW-Authenticate")) + if realm == "" { + return resp, nil + } + realmURL, err := url.Parse(realm) + if err != nil || realmURL.Host == "" { + if t.policy.Hardening() { + resp.Body.Close() + return nil, fmt.Errorf("registry '%s' requested authentication from an unparsable realm '%s'", req.URL.Host, realm) + } + return resp, nil + } + + if t.policy.allowsTokenServiceRealm(realmURL, req.URL) { + t.addTokenEndpoint(realmURL) + return resp, nil + } + + t.policy.record(&t.policy.diag.AuthLookupWouldBeBlocked, + fmt.Sprintf("Authentication lookup from registry '%s' to realm origin '%s' would be blocked by OCI auth hardening.", req.URL.Host, originOf(realmURL))) + if !t.policy.Hardening() { + t.addTokenEndpoint(realmURL) + return resp, nil + } + + hint := "" + if strings.EqualFold(realmURL.Scheme, "https") { + hint = fmt.Sprintf(" Use '--allow-cross-origin-auth-host %s=%s' to trust this registry-to-auth-host mapping.", req.URL.Host, realmURL.Host) + } + resp.Body.Close() + return nil, fmt.Errorf("registry '%s' requested authentication from untrusted realm '%s'.%s", req.URL.Host, realm, hint) +} + +func originOf(u *url.URL) string { + return strings.ToLower(u.Scheme + "://" + u.Host) +} + +// bearerRealm extracts realm="..." from a Bearer WWW-Authenticate challenge. +func bearerRealm(challenge string) string { + if challenge == "" { + return "" + } + scheme, params, found := strings.Cut(strings.TrimSpace(challenge), " ") + if !found || !strings.EqualFold(scheme, "Bearer") { + return "" + } + for _, part := range strings.Split(params, ",") { + key, value, ok := strings.Cut(strings.TrimSpace(part), "=") + if !ok || !strings.EqualFold(strings.TrimSpace(key), "realm") { + continue + } + return strings.Trim(strings.TrimSpace(value), `"`) + } + return "" +} diff --git a/internal/oci/authpolicy_test.go b/internal/oci/authpolicy_test.go new file mode 100644 index 0000000..4ff334a --- /dev/null +++ b/internal/oci/authpolicy_test.go @@ -0,0 +1,309 @@ +package oci + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/devcontainers/cli/internal/log" +) + +func mustURL(t *testing.T, raw string) *url.URL { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("parse %q: %v", raw, err) + } + return u +} + +func TestParseCrossOriginAuthHosts(t *testing.T) { + hosts, err := ParseCrossOriginAuthHosts([]string{"Registry.Example=Auth.Example:8443"}) + if err != nil { + t.Fatalf("parse: %v", err) + } + if !hosts["registry.example"]["auth.example:8443"] { + t.Fatalf("mapping not registered: %v", hosts) + } + + // Rejections mirror the reference CLI's messages, which the CLI surfaces verbatim. + for _, tc := range []struct{ entry, want string }{ + {"bad", "Invalid cross-origin auth host 'bad'. Expected '='."}, + {"=auth.example", "Invalid cross-origin auth host '=auth.example'. Expected '='."}, + {"registry.example=", "Invalid cross-origin auth host 'registry.example='. Expected '='."}, + {"a=b=c", "Invalid cross-origin auth host 'a=b=c'. Expected '='."}, + {"a/b=c", "Invalid authority 'a/b'."}, + {"a=b/c", "Invalid authority 'b/c'."}, + {"user@a=b", "Invalid authority 'user@a'."}, + } { + _, err := ParseCrossOriginAuthHosts([]string{tc.entry}) + if err == nil || err.Error() != tc.want { + t.Errorf("entry %q: got error %v, want %q", tc.entry, err, tc.want) + } + } +} + +func TestAllowsTokenServiceRealm(t *testing.T) { + policy, err := NewAuthPolicy(true, []string{"registry.example=auth.example"}, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + cases := []struct { + name string + realm, regist string + want bool + }{ + {"same authority https", "https://ghcr.io/token", "https://ghcr.io", true}, + {"same authority http on localhost", "http://localhost:5000/token", "http://localhost:5000", true}, + {"same authority http off localhost", "http://registry.example/token", "http://registry.example", false}, + {"built-in docker hub mapping", "https://auth.docker.io/token", "https://registry-1.docker.io", true}, + {"built-in gitlab mapping", "https://gitlab.com/jwt/auth", "https://registry.gitlab.com", true}, + {"configured mapping", "https://auth.example/token", "https://registry.example", true}, + {"configured mapping is not symmetric", "https://registry.example/token", "https://auth.example", false}, + {"untrusted cross origin", "https://evil.example/token", "https://ghcr.io", false}, + {"cross origin over http", "http://auth.example/token", "https://registry.example", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := policy.allowsTokenServiceRealm(mustURL(t, tc.realm), mustURL(t, tc.regist)) + if got != tc.want { + t.Errorf("allowsTokenServiceRealm(%q, %q) = %v, want %v", tc.realm, tc.regist, got, tc.want) + } + }) + } +} + +func TestIsRegistryOriginTreatsDockerHubAuthoritiesAsOne(t *testing.T) { + if !isRegistryOrigin(mustURL(t, "https://registry-1.docker.io"), mustURL(t, "https://docker.io")) { + t.Error("Docker Hub authorities should compare as the same origin") + } + if isRegistryOrigin(mustURL(t, "https://ghcr.io"), mustURL(t, "https://docker.io")) { + t.Error("unrelated registries must not compare as the same origin") + } + if !isRegistryOrigin(mustURL(t, "https://ghcr.io:443"), mustURL(t, "https://ghcr.io")) { + t.Error("the default port must be normalized") + } +} + +// roundTrip drives the policy transport against a fake registry response. +func roundTrip(t *testing.T, policy *AuthPolicy, registryScheme, registryHost string, handler http.HandlerFunc, requestPath string) (*http.Response, error) { + t.Helper() + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + host := registryHost + if host == "" { + host = strings.TrimPrefix(srv.URL, "http://") + } + transport := newAuthPolicyTransport(http.DefaultTransport, policy, registryScheme, host) + req, err := http.NewRequest(http.MethodGet, srv.URL+requestPath, nil) + if err != nil { + t.Fatalf("request: %v", err) + } + return transport.RoundTrip(req) +} + +func TestAuthLookupDiagnosticAndHardening(t *testing.T) { + challenge := func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer realm="https://evil.example/token",service="registry"`) + w.WriteHeader(http.StatusUnauthorized) + } + + // Hardening off: the untrusted realm is only reported, the response passes through. + policy, err := NewAuthPolicy(false, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + resp, err := roundTrip(t, policy, "http", "", challenge, "/v2/x/manifests/1") + if err != nil { + t.Fatalf("round trip: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", resp.StatusCode) + } + if diag := policy.Diagnostics(); !diag.AuthLookupWouldBeBlocked { + t.Errorf("authLookupWouldBeBlocked not recorded: %+v", diag) + } + + // Hardening on: the same challenge fails the request instead. + hardened, err := NewAuthPolicy(true, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + if _, err := roundTrip(t, hardened, "http", "", challenge, "/v2/x/manifests/1"); err == nil { + t.Fatal("hardened round trip should fail on an untrusted realm") + } else if !strings.Contains(err.Error(), "untrusted realm") || + !strings.Contains(err.Error(), "--allow-cross-origin-auth-host") { + t.Errorf("error = %v, want the untrusted-realm error with the allow hint", err) + } + if diag := hardened.Diagnostics(); !diag.AuthLookupWouldBeBlocked { + t.Errorf("authLookupWouldBeBlocked not recorded under hardening: %+v", diag) + } +} + +func TestSameAuthorityRealmIsAllowedAndNotReported(t *testing.T) { + policy, err := NewAuthPolicy(true, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + var srvHost string + handler := func(w http.ResponseWriter, r *http.Request) { + srvHost = r.Host + w.Header().Set("WWW-Authenticate", `Bearer realm="http://localhost:`+strings.SplitN(r.Host, ":", 2)[1]+`/token",service="registry"`) + w.WriteHeader(http.StatusUnauthorized) + } + // httptest serves on 127.0.0.1; the realm points at localhost on the same port, + // so force the registry host to the localhost form the realm uses. + srv := httptest.NewServer(http.HandlerFunc(handler)) + defer srv.Close() + port := strings.SplitN(strings.TrimPrefix(srv.URL, "http://"), ":", 2)[1] + transport := newAuthPolicyTransport(http.DefaultTransport, policy, "http", "localhost:"+port) + req, err := http.NewRequest(http.MethodGet, "http://localhost:"+port+"/v2/x/manifests/1", nil) + if err != nil { + t.Fatalf("request: %v", err) + } + resp, err := transport.RoundTrip(req) + if err != nil { + t.Fatalf("round trip: %v (host %q)", err, srvHost) + } + resp.Body.Close() + if diag := policy.Diagnostics(); diag != (AuthDiagnostics{}) { + t.Errorf("no diagnostic expected for a same-authority realm, got %+v", diag) + } + if !transport.isTokenEndpoint(mustURL(t, "http://localhost:"+port+"/token")) { + t.Error("the accepted realm should be registered as a token endpoint") + } +} + +func TestRegistryRedirectDiagnostic(t *testing.T) { + policy, err := NewAuthPolicy(false, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + // The challenge arrives from an origin other than the ref's registry. + _, err = roundTrip(t, policy, "https", "ghcr.io", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }, "/v2/x/manifests/1") + if err != nil { + t.Fatalf("round trip: %v", err) + } + if diag := policy.Diagnostics(); !diag.RegistryRedirectWouldPreventCredentialForwarding { + t.Errorf("registryRedirectWouldPreventCredentialForwarding not recorded: %+v", diag) + } +} + +func TestTokenEndpointRedirect(t *testing.T) { + handler := func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/token" { + http.Redirect(w, r, "https://elsewhere.example/token", http.StatusFound) + return + } + w.WriteHeader(http.StatusOK) + } + + // Hardening off: the redirect is recorded and followed. + policy, err := NewAuthPolicy(false, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + srv := httptest.NewServer(http.HandlerFunc(handler)) + defer srv.Close() + host := strings.TrimPrefix(srv.URL, "http://") + transport := newAuthPolicyTransport(http.DefaultTransport, policy, "http", host) + transport.addTokenEndpoint(mustURL(t, srv.URL+"/token")) + req, _ := http.NewRequest(http.MethodGet, srv.URL+"/token", nil) + resp, err := transport.RoundTrip(req) + if err != nil { + t.Fatalf("round trip: %v", err) + } + resp.Body.Close() + if diag := policy.Diagnostics(); !diag.AuthServerRedirect { + t.Errorf("authServerRedirect not recorded: %+v", diag) + } + + // Hardening on: a redirecting token endpoint fails the token request. + hardened, err := NewAuthPolicy(true, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + transport = newAuthPolicyTransport(http.DefaultTransport, hardened, "http", host) + transport.addTokenEndpoint(mustURL(t, srv.URL+"/token")) + req, _ = http.NewRequest(http.MethodGet, srv.URL+"/token", nil) + if _, err := transport.RoundTrip(req); err == nil { + t.Fatal("hardened token request should fail on a redirect") + } else if !strings.Contains(err.Error(), "redirected the token request") { + t.Errorf("error = %v, want the token-redirect error", err) + } +} + +func TestBearerRealm(t *testing.T) { + cases := map[string]string{ + `Bearer realm="https://ghcr.io/token",service="ghcr.io",scope="repository:x:pull"`: "https://ghcr.io/token", + `bearer service="x", realm="https://a.example/token"`: "https://a.example/token", + `Basic realm="https://ghcr.io/token"`: "", + `Bearer service="x"`: "", + ``: "", + } + for challenge, want := range cases { + if got := bearerRealm(challenge); got != want { + t.Errorf("bearerRealm(%q) = %q, want %q", challenge, got, want) + } + } +} + +func TestRefScheme(t *testing.T) { + cases := map[string]string{ + "ghcr.io/devcontainers/features/go:1": "https", + "localhost:5000/features/go:1": "http", + "127.0.0.1:5000/features/go:1": "https", + } + for input, want := range cases { + ref, err := ParseRef(input) + if err != nil { + t.Fatalf("parse %q: %v", input, err) + } + if got := ref.Scheme(); got != want { + t.Errorf("Scheme(%q) = %q, want %q", input, got, want) + } + } +} + +// TestClientHonorsAuthPolicyEndToEnd drives the policy through the real client: +// a registry that points its bearer challenge at an untrusted origin must fail the +// fetch under hardening, and must be reported (but tolerated) without it. +func TestClientHonorsAuthPolicyEndToEnd(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer realm="https://evil.example/token",service="registry"`) + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + + ref, err := ParseRef(strings.TrimPrefix(srv.URL, "http://") + "/devcontainers/features/go:1") + if err != nil { + t.Fatalf("parse ref: %v", err) + } + + hardened, err := NewAuthPolicy(true, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + if _, err := NewClientWithAuthPolicy(log.Null, map[string]string{"DOCKER_CONFIG": t.TempDir()}, hardened).FetchManifest(ref, ""); err == nil { + t.Error("fetch should fail when the registry points at an untrusted realm") + } + if diag := hardened.Diagnostics(); !diag.AuthLookupWouldBeBlocked { + t.Errorf("authLookupWouldBeBlocked not recorded through the client: %+v", diag) + } + + // Without hardening the same challenge is only reported; the failure that + // follows is the ordinary auth failure, not the policy refusing the realm. + policy, err := NewAuthPolicy(false, nil, log.Null) + if err != nil { + t.Fatalf("policy: %v", err) + } + _, _ = NewClientWithAuthPolicy(log.Null, map[string]string{"DOCKER_CONFIG": t.TempDir()}, policy).FetchManifest(ref, "") + if diag := policy.Diagnostics(); !diag.AuthLookupWouldBeBlocked { + t.Errorf("authLookupWouldBeBlocked not recorded with hardening off: %+v", diag) + } +} diff --git a/internal/oci/client.go b/internal/oci/client.go index 61da644..058533e 100644 --- a/internal/oci/client.go +++ b/internal/oci/client.go @@ -53,11 +53,22 @@ type Client struct { // certs (NODE_EXTRA_CA_CERTS/SSL_CERT_FILE), so registry access behaves like // the plain httpx path — including behind a TLS-intercepting proxy. retryClient *http.Client + // authPolicy carries the invocation-wide OCI authentication policy + // (--oci-auth-hardening, trusted cross-origin auth hosts) and accumulates the + // auth diagnostics reported in the command's JSON output. + authPolicy *AuthPolicy } // NewClient creates an OCI client. Auth and retries are handled by oras-go (see // repository()); the HTTP transport is the shared proxy/CA-aware transport. func NewClient(logger log.Logger, env map[string]string) *Client { + return NewClientWithAuthPolicy(logger, env, DefaultAuthPolicy()) +} + +// NewClientWithAuthPolicy is NewClient with an explicit OCI auth policy, so every +// client built for one CLI invocation shares its hardening settings and +// diagnostics. +func NewClientWithAuthPolicy(logger log.Logger, env map[string]string, policy *AuthPolicy) *Client { base := httpx.NewTransport() // Cut off a registry that connects but never sends response headers. base.ResponseHeaderTimeout = responseHeaderTimeout @@ -66,6 +77,7 @@ func NewClient(logger log.Logger, env map[string]string) *Client { env: env, authCache: auth.NewCache(), retryClient: &http.Client{Transport: retry.NewTransport(base)}, + authPolicy: policy, } } diff --git a/internal/oci/orasclient.go b/internal/oci/orasclient.go index 195c28c..6f7efe9 100644 --- a/internal/oci/orasclient.go +++ b/internal/oci/orasclient.go @@ -3,6 +3,7 @@ package oci import ( "context" "encoding/base64" + "net/http" "strings" "oras.land/oras-go/v2/registry/remote" @@ -46,6 +47,24 @@ func (c *Client) repository(ref *Ref) (*remote.Repository, error) { if httpClient == nil { httpClient = retry.DefaultClient } + // Apply the OCI auth policy (realm pinning, token-redirect refusal, auth + // diagnostics) to this repository's traffic. The wrapper sits above the + // retrying transport so it observes the response the registry settled on. + policy := c.authPolicy + if policy == nil { + policy = DefaultAuthPolicy() + } + scheme := "https" + if repo.PlainHTTP { + scheme = "http" + } + baseTransport := httpClient.Transport + if baseTransport == nil { + baseTransport = http.DefaultTransport + } + policyClient := *httpClient + policyClient.Transport = newAuthPolicyTransport(baseTransport, policy, scheme, ref.Registry) + httpClient = &policyClient repo.Client = &auth.Client{ Client: httpClient, Cache: cache, diff --git a/internal/oci/ref.go b/internal/oci/ref.go index 6486255..2b298e4 100644 --- a/internal/oci/ref.go +++ b/internal/oci/ref.go @@ -33,6 +33,21 @@ type Ref struct { Digest string // "sha256:..." (empty if tag) } +// Scheme is the transport the registry is addressed over, mirroring the TS +// getRegistryScheme (containerCollectionsOCI.ts): plain HTTP for a localhost +// registry, HTTPS otherwise. It is part of the `read-configuration` output +// (featuresConfiguration.featureSets[].sourceInformation.featureRef.scheme). +func (r *Ref) Scheme() string { + host := r.Registry + if i := strings.IndexByte(host, ':'); i >= 0 { + host = host[:i] + } + if strings.EqualFold(host, "localhost") { + return "http" + } + return "https" +} + // CollectionRef represents a collection metadata artifact reference. // e.g., ghcr.io/devcontainers/features:latest type CollectionRef struct { diff --git a/reference b/reference index f683c29..5dc7533 160000 --- a/reference +++ b/reference @@ -1 +1 @@ -Subproject commit f683c29f64a20109b4453e5149807e390ff65133 +Subproject commit 5dc7533314b5ba7ec3875c30143dfe1aec644870 From 7d4024175005f88cce59cb7c1fbba5651a675c52 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 20 Sep 2026 21:52:52 -0300 Subject: [PATCH 3/4] refactor(docker): replace MongoDB with Redis in docker-compose configurations --- docs/parity/parity-matrix.yaml | 14 +++++++------- .../app-port/.devcontainer/devcontainer.json | 2 +- .../.devcontainer/docker-compose.yml | 12 +++++------- .../.devcontainer/docker-compose.yml | 12 +++++------- .../.devcontainer/docker-compose.yml | 12 +++++------- .../.devcontainer/docker-compose.yml | 12 +++++------- .../.devcontainer/docker-compose.yml | 12 +++++------- 7 files changed, 33 insertions(+), 43 deletions(-) diff --git a/docs/parity/parity-matrix.yaml b/docs/parity/parity-matrix.yaml index 480a43e..62b396c 100644 --- a/docs/parity/parity-matrix.yaml +++ b/docs/parity/parity-matrix.yaml @@ -1344,7 +1344,7 @@ initial_cases: docker_required: true network_required: true ts_cmd: "up --workspace-folder src/test/configs/app-port --buildkit=never --skip-post-create --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE} --remove-existing-container" - verify_cmd: "P=$(docker inspect --format '{{json .HostConfig.PortBindings}}' ${CONTAINER_ID}); echo \"$P\" | grep -q '\"HostPort\":\"3000\"' && echo \"$P\" | grep -q '\"HostPort\":\"8080\"' && printf ok" + verify_cmd: "P=$(docker inspect --format '{{json .HostConfig.PortBindings}}' ${CONTAINER_ID}); echo \"$P\" | grep -q '\"HostPort\":\"31300\"' && echo \"$P\" | grep -q '\"HostPort\":\"31808\"' && printf ok" cleanup_cmd: "IDS=$(docker ps -aq --filter label=parity.case=${PARITY_CASE_ID}-${PARITY_SIDE}); [ -n \"$IDS\" ] && docker rm -f $IDS >/dev/null 2>&1 || true" asserts: [exit_code, stdout_normalized] class: up-runtime @@ -1540,13 +1540,13 @@ initial_cases: docker_required: true network_required: true ts_cmd: "up --workspace-folder src/test/configs/compose-image-with-features --buildkit=never --omit-config-remote-env-from-metadata --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE} --remove-existing-container" - setup_cmd: "docker pull --platform linux/amd64 mongo:latest >/dev/null; echo DOCKER_DEFAULT_PLATFORM=linux/amd64" + setup_cmd: "docker pull --platform linux/amd64 redis:alpine >/dev/null; echo DOCKER_DEFAULT_PLATFORM=linux/amd64" verify_cmd: "test \"$(docker exec ${CONTAINER_ID} cat /postCreateCommand.txt)\" = \"Val: ENV\" && ! docker inspect --format '{{ index .Config.Labels \"devcontainer.metadata\" }}' ${CONTAINER_ID} | grep -q 'TEST_ESCAPING' && printf ok" cleanup_cmd: "docker compose --project-name ${COMPOSE_PROJECT_NAME} -f src/test/configs/compose-image-with-features/.devcontainer/docker-compose.yml down >/dev/null 2>&1 || true" asserts: [exit_code, stdout_normalized] class: up-runtime current_status: match - notes: "Unblocked by pinning the harness setup with `docker pull --platform linux/amd64 mongo:latest` before compose." + notes: "Unblocked by pinning the harness setup with `docker pull --platform linux/amd64 redis:alpine` (the fixture's db service) before compose." - id: up.compose-dockerfile-with-features-omit-config-remote-env-from-metadata-success lane: runtime @@ -1555,13 +1555,13 @@ initial_cases: docker_required: true network_required: true ts_cmd: "up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --omit-config-remote-env-from-metadata --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE} --remove-existing-container" - setup_cmd: "docker pull --platform linux/amd64 mongo:latest >/dev/null; echo DOCKER_DEFAULT_PLATFORM=linux/amd64" + setup_cmd: "docker pull --platform linux/amd64 redis:alpine >/dev/null; echo DOCKER_DEFAULT_PLATFORM=linux/amd64" verify_cmd: "test \"$(docker exec ${CONTAINER_ID} cat /postCreateCommand.txt)\" = \"Val: ENV\" && ! docker inspect --format '{{ index .Config.Labels \"devcontainer.metadata\" }}' ${CONTAINER_ID} | grep -q 'TEST_ESCAPING' && printf ok" cleanup_cmd: "docker compose --project-name ${COMPOSE_PROJECT_NAME} -f src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml down >/dev/null 2>&1 || true" asserts: [exit_code, stdout_normalized] class: up-runtime current_status: match - notes: "Closed out after aligning the metadata of Dockerfile+features builds; the setup keeps `docker pull --platform linux/amd64 mongo:latest` to avoid platform drift in compose." + notes: "Closed out after aligning the metadata of Dockerfile+features builds; the setup keeps `docker pull --platform linux/amd64 redis:alpine` (the fixture's db service) to avoid platform drift in compose." - id: up.compose-dockerfile-without-features-omit-config-remote-env-from-metadata-success lane: runtime @@ -1729,7 +1729,7 @@ initial_cases: docker_required: true network_required: true ts_cmd: "up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER}" - setup_cmd: "docker pull --platform linux/amd64 mongo:latest >/dev/null; USER_DATA_FOLDER=/tmp/parity-user-data-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf ${USER_DATA_FOLDER}; mkdir -p ${USER_DATA_FOLDER}; if [ \"${PARITY_SIDE}\" = \"ts\" ]; then ${PARITY_CLI_TS} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; else ${PARITY_CLI_GO} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; fi; ORIGINAL_CONTAINER_ID=$(docker ps -aq --filter label=com.docker.compose.project=compose-dockerfile-with-features_devcontainer --filter label=com.docker.compose.service=app | head -n1); docker compose --project-name compose-dockerfile-with-features_devcontainer stop >/dev/null; echo DOCKER_DEFAULT_PLATFORM=linux/amd64; echo USER_DATA_FOLDER=${USER_DATA_FOLDER}; echo ORIGINAL_CONTAINER_ID=${ORIGINAL_CONTAINER_ID}" + setup_cmd: "docker pull --platform linux/amd64 redis:alpine >/dev/null; USER_DATA_FOLDER=/tmp/parity-user-data-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf ${USER_DATA_FOLDER}; mkdir -p ${USER_DATA_FOLDER}; if [ \"${PARITY_SIDE}\" = \"ts\" ]; then ${PARITY_CLI_TS} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; else ${PARITY_CLI_GO} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; fi; ORIGINAL_CONTAINER_ID=$(docker ps -aq --filter label=com.docker.compose.project=compose-dockerfile-with-features_devcontainer --filter label=com.docker.compose.service=app | head -n1); docker compose --project-name compose-dockerfile-with-features_devcontainer stop >/dev/null; echo DOCKER_DEFAULT_PLATFORM=linux/amd64; echo USER_DATA_FOLDER=${USER_DATA_FOLDER}; echo ORIGINAL_CONTAINER_ID=${ORIGINAL_CONTAINER_ID}" verify_cmd: "test \"${CONTAINER_ID#$ORIGINAL_CONTAINER_ID}\" != \"${CONTAINER_ID}\" && test \"$(find ${USER_DATA_FOLDER}/docker-compose -maxdepth 1 -type f | wc -l | tr -d ' ')\" = \"2\" && find ${USER_DATA_FOLDER}/docker-compose -maxdepth 1 -type f -name 'docker-compose.devcontainer.build-*' | grep -q . && find ${USER_DATA_FOLDER}/docker-compose -maxdepth 1 -type f -name 'docker-compose.devcontainer.containerFeatures-*' | grep -q . && printf ok" cleanup_cmd: "docker compose --project-name compose-dockerfile-with-features_devcontainer -f src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml down >/dev/null 2>&1 || true; rm -rf ${USER_DATA_FOLDER}" asserts: [exit_code, stdout_normalized] @@ -1746,7 +1746,7 @@ initial_cases: docker_required: true network_required: true ts_cmd: "up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER}" - setup_cmd: "docker pull --platform linux/amd64 mongo:latest >/dev/null; USER_DATA_FOLDER=/tmp/parity-user-data-reset-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf ${USER_DATA_FOLDER}; mkdir -p ${USER_DATA_FOLDER}; if [ \"${PARITY_SIDE}\" = \"ts\" ]; then ${PARITY_CLI_TS} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; else ${PARITY_CLI_GO} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; fi; ORIGINAL_CONTAINER_ID=$(docker ps -aq --filter label=com.docker.compose.project=compose-dockerfile-with-features_devcontainer --filter label=com.docker.compose.service=app | head -n1); docker compose --project-name compose-dockerfile-with-features_devcontainer stop >/dev/null; rm -rf ${USER_DATA_FOLDER}; mkdir -p ${USER_DATA_FOLDER}; echo DOCKER_DEFAULT_PLATFORM=linux/amd64; echo USER_DATA_FOLDER=${USER_DATA_FOLDER}; echo ORIGINAL_CONTAINER_ID=${ORIGINAL_CONTAINER_ID}" + setup_cmd: "docker pull --platform linux/amd64 redis:alpine >/dev/null; USER_DATA_FOLDER=/tmp/parity-user-data-reset-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf ${USER_DATA_FOLDER}; mkdir -p ${USER_DATA_FOLDER}; if [ \"${PARITY_SIDE}\" = \"ts\" ]; then ${PARITY_CLI_TS} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; else ${PARITY_CLI_GO} up --workspace-folder src/test/configs/compose-Dockerfile-with-features --buildkit=never --user-data-folder ${USER_DATA_FOLDER} --remove-existing-container >/dev/null; fi; ORIGINAL_CONTAINER_ID=$(docker ps -aq --filter label=com.docker.compose.project=compose-dockerfile-with-features_devcontainer --filter label=com.docker.compose.service=app | head -n1); docker compose --project-name compose-dockerfile-with-features_devcontainer stop >/dev/null; rm -rf ${USER_DATA_FOLDER}; mkdir -p ${USER_DATA_FOLDER}; echo DOCKER_DEFAULT_PLATFORM=linux/amd64; echo USER_DATA_FOLDER=${USER_DATA_FOLDER}; echo ORIGINAL_CONTAINER_ID=${ORIGINAL_CONTAINER_ID}" verify_cmd: "docker inspect ${CONTAINER_ID} >/dev/null && printf ok" cleanup_cmd: "docker compose --project-name compose-dockerfile-with-features_devcontainer -f src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml down >/dev/null 2>&1 || true; rm -rf ${USER_DATA_FOLDER}" asserts: [exit_code, stdout_normalized] diff --git a/src/test/configs/app-port/.devcontainer/devcontainer.json b/src/test/configs/app-port/.devcontainer/devcontainer.json index 7a182a1..c79ae3c 100644 --- a/src/test/configs/app-port/.devcontainer/devcontainer.json +++ b/src/test/configs/app-port/.devcontainer/devcontainer.json @@ -1,4 +1,4 @@ { "image": "mcr.microsoft.com/devcontainers/base:ubuntu", - "appPort": [3000, "8080:80"] + "appPort": [31300, "31808:80"] } diff --git a/src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml b/src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml index 3833580..4ed7630 100644 --- a/src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml +++ b/src/test/configs/compose-Dockerfile-with-features/.devcontainer/docker-compose.yml @@ -25,18 +25,16 @@ services: # (Adding the "ports" property to this file will not forward from a Codespace.) db: - image: mongo:latest + image: redis:alpine restart: unless-stopped volumes: - - mongodb-data:/data/db + - db-data:/data # Uncomment to change startup options # environment: - # MONGO_INITDB_ROOT_USERNAME: root - # MONGO_INITDB_ROOT_PASSWORD: example - # MONGO_INITDB_DATABASE: your-database-here + # SOME_DB_OPTION: value - # Add "forwardPorts": ["27017"] to **devcontainer.json** to forward MongoDB locally. + # Add "forwardPorts": ["6379"] to **devcontainer.json** to forward the db locally. # (Adding the "ports" property to this file will not forward from a Codespace.) volumes: - mongodb-data: null + db-data: null diff --git a/src/test/configs/compose-Dockerfile-with-target/.devcontainer/docker-compose.yml b/src/test/configs/compose-Dockerfile-with-target/.devcontainer/docker-compose.yml index b9abcda..76df2a1 100644 --- a/src/test/configs/compose-Dockerfile-with-target/.devcontainer/docker-compose.yml +++ b/src/test/configs/compose-Dockerfile-with-target/.devcontainer/docker-compose.yml @@ -26,18 +26,16 @@ services: # (Adding the "ports" property to this file will not forward from a Codespace.) db: - image: mongo:latest + image: redis:alpine restart: unless-stopped volumes: - - mongodb-data:/data/db + - db-data:/data # Uncomment to change startup options # environment: - # MONGO_INITDB_ROOT_USERNAME: root - # MONGO_INITDB_ROOT_PASSWORD: example - # MONGO_INITDB_DATABASE: your-database-here + # SOME_DB_OPTION: value - # Add "forwardPorts": ["27017"] to **devcontainer.json** to forward MongoDB locally. + # Add "forwardPorts": ["6379"] to **devcontainer.json** to forward the db locally. # (Adding the "ports" property to this file will not forward from a Codespace.) volumes: - mongodb-data: null + db-data: null diff --git a/src/test/configs/compose-Dockerfile-without-features/.devcontainer/docker-compose.yml b/src/test/configs/compose-Dockerfile-without-features/.devcontainer/docker-compose.yml index f69e7e9..2874621 100644 --- a/src/test/configs/compose-Dockerfile-without-features/.devcontainer/docker-compose.yml +++ b/src/test/configs/compose-Dockerfile-without-features/.devcontainer/docker-compose.yml @@ -25,18 +25,16 @@ services: # (Adding the "ports" property to this file will not forward from a Codespace.) db: - image: mongo:latest + image: redis:alpine restart: unless-stopped volumes: - - mongodb-data:/data/db + - db-data:/data # Uncomment to change startup options # environment: - # MONGO_INITDB_ROOT_USERNAME: root - # MONGO_INITDB_ROOT_PASSWORD: example - # MONGO_INITDB_DATABASE: your-database-here + # SOME_DB_OPTION: value - # Add "forwardPorts": ["27017"] to **devcontainer.json** to forward MongoDB locally. + # Add "forwardPorts": ["6379"] to **devcontainer.json** to forward the db locally. # (Adding the "ports" property to this file will not forward from a Codespace.) volumes: - mongodb-data: null + db-data: null diff --git a/src/test/configs/compose-image-with-features/.devcontainer/docker-compose.yml b/src/test/configs/compose-image-with-features/.devcontainer/docker-compose.yml index 07185b2..e28ee17 100644 --- a/src/test/configs/compose-image-with-features/.devcontainer/docker-compose.yml +++ b/src/test/configs/compose-image-with-features/.devcontainer/docker-compose.yml @@ -18,18 +18,16 @@ services: # (Adding the "ports" property to this file will not forward from a Codespace.) db: - image: mongo:latest + image: redis:alpine restart: unless-stopped volumes: - - mongodb-data:/data/db + - db-data:/data # Uncomment to change startup options # environment: - # MONGO_INITDB_ROOT_USERNAME: root - # MONGO_INITDB_ROOT_PASSWORD: example - # MONGO_INITDB_DATABASE: your-database-here + # SOME_DB_OPTION: value - # Add "forwardPorts": ["27017"] to **devcontainer.json** to forward MongoDB locally. + # Add "forwardPorts": ["6379"] to **devcontainer.json** to forward the db locally. # (Adding the "ports" property to this file will not forward from a Codespace.) volumes: - mongodb-data: null + db-data: null diff --git a/src/test/configs/compose-image-without-features/.devcontainer/docker-compose.yml b/src/test/configs/compose-image-without-features/.devcontainer/docker-compose.yml index aae55f6..1966301 100644 --- a/src/test/configs/compose-image-without-features/.devcontainer/docker-compose.yml +++ b/src/test/configs/compose-image-without-features/.devcontainer/docker-compose.yml @@ -18,18 +18,16 @@ services: # (Adding the "ports" property to this file will not forward from a Codespace.) db: - image: mongo:latest + image: redis:alpine restart: unless-stopped volumes: - - mongodb-data:/data/db + - db-data:/data # Uncomment to change startup options # environment: - # MONGO_INITDB_ROOT_USERNAME: root - # MONGO_INITDB_ROOT_PASSWORD: example - # MONGO_INITDB_DATABASE: your-database-here + # SOME_DB_OPTION: value - # Add "forwardPorts": ["27017"] to **devcontainer.json** to forward MongoDB locally. + # Add "forwardPorts": ["6379"] to **devcontainer.json** to forward the db locally. # (Adding the "ports" property to this file will not forward from a Codespace.) volumes: - mongodb-data: null + db-data: null From e94cff18d42a32d22ce6e587a24783ceb9b38ce0 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 20 Sep 2026 21:53:39 -0300 Subject: [PATCH 4/4] ci: set least-privilege GITHUB_TOKEN permissions for the CI workflow CodeQL reported "Workflow does not contain permissions" for every job of go-cli.yml except lint-and-test: without a `permissions` block the token inherits the repository default, which may include write scopes the jobs never need. Declare `contents: read` once at the workflow level. Every job only checks out the repo and reads the API (setup-task's repo-token), and artifact upload/download uses the Actions runtime token, so read is sufficient; a future job that needs more widens it for itself. TestWorkflowsDeclarePermissions pins the rule for all workflows (top-level or per-job), and TestGoCLIWorkflowIsReadOnlyByDefault pins this grant, so a job added later cannot silently inherit write access. Both fail without the block: the first names all seven unprotected jobs. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/go-cli.yml | 8 ++ internal/cli/workflow_permissions_test.go | 91 +++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 internal/cli/workflow_permissions_test.go diff --git a/.github/workflows/go-cli.yml b/.github/workflows/go-cli.yml index 50370c5..e3f2196 100644 --- a/.github/workflows/go-cli.yml +++ b/.github/workflows/go-cli.yml @@ -11,6 +11,14 @@ on: # push/PR only the cases affected by the change run (see parity-runtime). - cron: "0 6 * * *" +# Least privilege for every job in this workflow: the jobs only check out the repo +# and read the API (setup-task's repo-token). A job that needs more must widen it +# for itself. Without this default, GITHUB_TOKEN falls back to the repository's +# (possibly write) default — see the CodeQL "workflow does not contain permissions" +# rule, which TestWorkflowsDeclarePermissions pins. +permissions: + contents: read + jobs: lint-and-test: runs-on: ubuntu-latest diff --git a/internal/cli/workflow_permissions_test.go b/internal/cli/workflow_permissions_test.go new file mode 100644 index 0000000..cc27ae2 --- /dev/null +++ b/internal/cli/workflow_permissions_test.go @@ -0,0 +1,91 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + + "gopkg.in/yaml.v3" +) + +// TestWorkflowsDeclarePermissions pins least privilege for GITHUB_TOKEN: every +// workflow must set `permissions`, either at the top level (covering all of its +// jobs) or on each job. Without it the token inherits the repository default, +// which CodeQL reports as "Workflow does not contain permissions" — six jobs of +// go-cli.yml were flagged that way. +// +// A Go test cannot exercise CI, so this asserts the workflow files themselves. +func TestWorkflowsDeclarePermissions(t *testing.T) { + dir := filepath.Join("..", "..", ".github", "workflows") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read %s: %v", dir, err) + } + + type workflow struct { + Permissions yaml.Node `yaml:"permissions"` + Jobs map[string]struct { + Permissions yaml.Node `yaml:"permissions"` + // A job that only calls a reusable workflow declares `uses`; its + // permissions belong to the called workflow. + Uses string `yaml:"uses"` + } `yaml:"jobs"` + } + + seen := 0 + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || (filepath.Ext(name) != ".yml" && filepath.Ext(name) != ".yaml") { + continue + } + seen++ + data, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + var wf workflow + if err := yaml.Unmarshal(data, &wf); err != nil { + t.Fatalf("parse %s: %v", name, err) + } + if !wf.Permissions.IsZero() { + continue // a top-level block covers every job + } + if len(wf.Jobs) == 0 { + t.Errorf("%s: no jobs found (parse problem?)", name) + continue + } + for job, spec := range wf.Jobs { + if spec.Uses == "" && spec.Permissions.IsZero() { + t.Errorf("%s: job %q declares no permissions — add a job-level `permissions:` block, or a top-level one for the whole workflow", name, job) + } + } + } + if seen == 0 { + t.Fatalf("no workflow files found under %s", dir) + } +} + +// TestGoCLIWorkflowIsReadOnlyByDefault pins the specific grant: the CI workflow's +// default is read-only, so a job added later cannot silently inherit write access. +func TestGoCLIWorkflowIsReadOnlyByDefault(t *testing.T) { + path := filepath.Join("..", "..", ".github", "workflows", "go-cli.yml") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + var wf struct { + Permissions map[string]string `yaml:"permissions"` + } + if err := yaml.Unmarshal(data, &wf); err != nil { + t.Fatalf("parse %s: %v", path, err) + } + want := map[string]string{"contents": "read"} + if len(wf.Permissions) != len(want) { + t.Fatalf("permissions = %v, want %v", wf.Permissions, want) + } + for scope, level := range want { + if wf.Permissions[scope] != level { + t.Errorf("permissions[%q] = %q, want %q", scope, wf.Permissions[scope], level) + } + } +}