diff --git a/.github/workflows/go-cli.yml b/.github/workflows/go-cli.yml index 04c41af..0ce934e 100644 --- a/.github/workflows/go-cli.yml +++ b/.github/workflows/go-cli.yml @@ -16,7 +16,6 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - code-quality: write steps: - uses: actions/checkout@v7 - uses: actions/setup-go@v7 @@ -39,19 +38,6 @@ jobs: - run: task build - run: ./devcontainer --version - # Convert the Go coverage profile (coverage.out) to Cobertura XML for - # GitHub's native Code Quality feature. - - name: Convert coverage to Cobertura XML - run: | - go install github.com/boumenot/gocover-cobertura@latest - gocover-cobertura < coverage.out > coverage.xml - - name: Upload coverage report - uses: actions/upload-code-coverage@v1 - with: - file: coverage.xml - language: Go - label: code-coverage/go - # Binary covdata for the cross-lane merge (coverage-report job). - uses: actions/upload-artifact@v7 if: always() diff --git a/internal/cli/code_quality_workflow_test.go b/internal/cli/code_quality_workflow_test.go new file mode 100644 index 0000000..80f376e --- /dev/null +++ b/internal/cli/code_quality_workflow_test.go @@ -0,0 +1,23 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestGoCLIWorkflowDoesNotUploadDisabledCodeQualityCoverage(t *testing.T) { + workflowPath := filepath.Join("..", "..", ".github", "workflows", "go-cli.yml") + data, err := os.ReadFile(workflowPath) + if err != nil { + t.Fatalf("read %s: %v", workflowPath, err) + } + + workflow := string(data) + for _, disabledSetting := range []string{"actions/upload-code-coverage", "code-quality: write"} { + if strings.Contains(workflow, disabledSetting) { + t.Errorf("%s still contains disabled Code Quality setting %q", workflowPath, disabledSetting) + } + } +} diff --git a/internal/cli/parity_matrix_helpers_test.go b/internal/cli/parity_matrix_helpers_test.go index 6d330f6..de3de49 100644 --- a/internal/cli/parity_matrix_helpers_test.go +++ b/internal/cli/parity_matrix_helpers_test.go @@ -168,6 +168,22 @@ func TestExtractCLIResultEnv_EmbeddedJSON(t *testing.T) { } } +func TestNormalizeRequired(t *testing.T) { + got := normalizeRequired("One of --workspace-folder or --workspace-folder-data is required.") + want := "workspace-folder,workspace-folder-data" + if got != want { + t.Fatalf("normalizeRequired() = %q, want %q", got, want) + } +} + +func TestComposeProjectName(t *testing.T) { + got := composeProjectName("Build_Feature-1 / alpine") + want := "dcbuild_feature-1alpine" + if got != want { + t.Fatalf("composeProjectName() = %q, want %q", got, want) + } +} + // TestInShardPartitions proves the shard split is a proper partition: with N // shards, every case is claimed by exactly one shard and the union is the whole // set (no case dropped, none run twice). diff --git a/internal/cli/parity_matrix_test.go b/internal/cli/parity_matrix_test.go index 7900c14..e56410a 100644 --- a/internal/cli/parity_matrix_test.go +++ b/internal/cli/parity_matrix_test.go @@ -921,6 +921,9 @@ var reChoiceYargs = regexp.MustCompile(`(?m)Argument:\s*([^,]+),\s*Given:\s*"([^ var reChoiceGo = regexp.MustCompile(`(?m)Invalid value "([^"]+)" for --([^.\s]+)\.\s*Choose from:\s*(.+)$`) var reInvalidMode = regexp.MustCompile(`(?m)Invalid mode "([^"]+)".*Choose from:\s*(.+)$`) var reSetupEnv = regexp.MustCompile(`^([A-Za-z_][A-Za-z0-9_]*)=(.*)$`) +var reRequiredPrefix = regexp.MustCompile(`(?i)^One of\s+`) +var reRequiredSuffix = regexp.MustCompile(`(?i)\s+is required\.?$`) +var reRequiredSplit = regexp.MustCompile(`\s+or\s+|,\s*`) func matchChoiceYargs(text string) string { match := reChoiceYargs.FindStringSubmatch(text) @@ -968,9 +971,9 @@ func normalizeChoices(raw string) string { } func normalizeRequired(raw string) string { - raw = regexp.MustCompile(`(?i)^One of\s+`).ReplaceAllString(raw, "") - raw = regexp.MustCompile(`(?i)\s+is required\.?$`).ReplaceAllString(raw, "") - parts := regexp.MustCompile(`\s+or\s+|,\s*`).Split(raw, -1) + raw = reRequiredPrefix.ReplaceAllString(raw, "") + raw = reRequiredSuffix.ReplaceAllString(raw, "") + parts := reRequiredSplit.Split(raw, -1) var clean []string for _, p := range parts { p = strings.TrimSpace(strings.TrimPrefix(p, "--")) @@ -1089,13 +1092,17 @@ func composeProjectName(caseID string) string { var b strings.Builder b.WriteString("dc") for _, r := range strings.ToLower(caseID) { - if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + if isComposeProjectNameRune(r) { b.WriteRune(r) } } return b.String() } +func isComposeProjectNameRune(r rune) bool { + return (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' +} + func sanitizeEnvValue(value string) string { replacer := strings.NewReplacer("/", "-", " ", "-", ":", "-", "\t", "-", "\n", "-") return replacer.Replace(value) diff --git a/internal/templates/apply.go b/internal/templates/apply.go index 2c31d1d..112be4b 100644 --- a/internal/templates/apply.go +++ b/internal/templates/apply.go @@ -60,12 +60,19 @@ func FetchAndApply(params ApplyParams, selected SelectedTemplate) ([]string, err return nil, fmt.Errorf("fetch template blob: %w", err) } - // Extract to temp dir - tmpDir := params.TmpDir - if tmpDir == "" { - tmpDir = os.TempDir() + // Extract to a unique temporary directory when the caller did not provide + // one. Caller-provided directories retain the stable per-template layout and + // remain caller-owned. + var extractDir string + if params.TmpDir == "" { + extractDir, err = os.MkdirTemp("", "devcontainer-template-") + if err != nil { + return nil, fmt.Errorf("create extract dir: %w", err) + } + defer os.RemoveAll(extractDir) + } else { + extractDir = filepath.Join(params.TmpDir, "template-"+ref.ID) } - extractDir := filepath.Join(tmpDir, "template-"+ref.ID) if err := fsys.MkdirAll(extractDir); err != nil { return nil, fmt.Errorf("create extract dir: %w", err) } @@ -191,7 +198,9 @@ func mergeFeatures(fsys pfs.FS, workspaceFolder string, featureOpts []TemplateFe return fmt.Errorf("parse %s: %w", configPath, stdErr) } var config map[string]json.RawMessage - json.Unmarshal(stdData, &config) + if err := json.Unmarshal(stdData, &config); err != nil { + return fmt.Errorf("unmarshal %s: %w", configPath, err) + } existing := map[string]bool{} _, hasFeatures := config["features"] if hasFeatures { @@ -262,8 +271,12 @@ func applyOptionDefaults(fsys pfs.FS, extractDir string, userOptions map[string] if err != nil { return merged } + standardized, err := hujson.Standardize(data) + if err != nil { + return merged + } var meta TemplateMetadata - if err := json.Unmarshal(data, &meta); err != nil { + if err := json.Unmarshal(standardized, &meta); err != nil { return merged } for key, raw := range meta.Options { diff --git a/internal/templates/apply_fetch_test.go b/internal/templates/apply_fetch_test.go index 04b8bb0..8c21df0 100644 --- a/internal/templates/apply_fetch_test.go +++ b/internal/templates/apply_fetch_test.go @@ -194,6 +194,76 @@ func TestFetchAndApply_Success(t *testing.T) { } } +func TestFetchAndApply_DefaultTempDirIsRemoved(t *testing.T) { + tests := []struct { + name string + blob func(*testing.T) []byte + wantErr bool + }{ + { + name: "success", + blob: func(t *testing.T) []byte { + return buildTemplateTarGz(t, templateEntries()) + }, + }, + { + name: "extraction error", + blob: func(*testing.T) []byte { + return []byte("not a valid gzip tarball") + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempRoot := t.TempDir() + t.Setenv("TMPDIR", tempRoot) + params := ApplyParams{ + OCIClient: &fakeTemplateRegistry{blob: tt.blob(t)}, + FS: pfs.OSFS{}, + Logger: log.Null, + WorkspaceFolder: t.TempDir(), + } + + _, err := FetchAndApply(params, SelectedTemplate{ID: "ghcr.io/devcontainers/templates/sample:1"}) + if (err != nil) != tt.wantErr { + t.Fatalf("FetchAndApply() error = %v, wantErr %v", err, tt.wantErr) + } + entries, readErr := os.ReadDir(tempRoot) + if readErr != nil { + t.Fatal(readErr) + } + if len(entries) != 0 { + t.Fatalf("default temp root contains %v after FetchAndApply", entries) + } + }) + } +} + +func TestFetchAndApply_CallerTempDirIsPreserved(t *testing.T) { + tempRoot := t.TempDir() + params := ApplyParams{ + OCIClient: &fakeTemplateRegistry{blob: buildTemplateTarGz(t, templateEntries())}, + FS: pfs.OSFS{}, + Logger: log.Null, + WorkspaceFolder: t.TempDir(), + TmpDir: tempRoot, + } + + _, err := FetchAndApply(params, SelectedTemplate{ID: "ghcr.io/devcontainers/templates/sample:1"}) + if err != nil { + t.Fatalf("FetchAndApply: %v", err) + } + entries, err := os.ReadDir(tempRoot) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 || entries[0].Name() != "template-sample" { + t.Fatalf("caller temp root contains %v, want preserved template-sample directory", entries) + } +} + // TestFetchAndApply_PartialWorkspaceWrite covers the partial-write risk: a WriteFile // that fails mid-Walk. The first workspace file is written, the second fails, // and the error must propagate (wrapped) instead of silently leaving a partial diff --git a/internal/templates/apply_pure_test.go b/internal/templates/apply_pure_test.go index 83bfe7c..a9f4ef5 100644 --- a/internal/templates/apply_pure_test.go +++ b/internal/templates/apply_pure_test.go @@ -60,6 +60,25 @@ func TestApplyOptionDefaults(t *testing.T) { } } +func TestApplyOptionDefaults_JSONCMetadata(t *testing.T) { + dir := t.TempDir() + metadata := []byte(`{ + // Template metadata permits JSON with comments and trailing commas. + "id": "x", + "options": { + "imageVariant": { "type": "string", "default": "bookworm", }, + }, + }`) + if err := os.WriteFile(filepath.Join(dir, "devcontainer-template.json"), metadata, 0o644); err != nil { + t.Fatal(err) + } + + got := applyOptionDefaults(pfs.OSFS{}, dir, nil, log.Null) + if got["imageVariant"] != "bookworm" { + t.Fatalf("imageVariant = %q, want JSONC default %q", got["imageVariant"], "bookworm") + } +} + func TestApplyOptionDefaults_NoMetadata(t *testing.T) { // Missing devcontainer-template.json → returns the user options unchanged. dir := t.TempDir()