From e488cf4cd3f06195ff2fcc9d9f56e24b20ade9b5 Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 14:40:19 +0800 Subject: [PATCH 01/50] feat(qualitygate): enforce six-layer package dependency boundaries Add a data-driven architecture layering test that builds the full import graph (go list -json -tags authsidecar) and evaluates six rules: - extension must not depend on internal (transitive; keeps it extractable as a standalone SDK module) - events must not depend on shortcuts (transitive) - shortcuts must not directly import auth/keychain/credential/client/vfs (direct; must go through the shortcuts/common RuntimeContext gate) - cmd subpackages must not import shortcuts (assembly point + cmd/auth only) - errs must stay a leaf - internal must not depend on cmd/shortcuts/events Pre-existing violations are seeded into layering-edges.txt (37 rows). The test rejects any unseeded violation (new debt) and any stale row (removed debt), and CI locks the effective row count to only ever decrease. Removes the tautological circular-dependency check from arch-audit.yml, since Go already forbids import cycles at compile time. --- .github/workflows/arch-audit.yml | 13 - .github/workflows/ci.yml | 14 + .../qualitygate/deptest/layering-edges.txt | 40 ++ internal/qualitygate/deptest/layering_test.go | 508 ++++++++++++++++++ 4 files changed, 562 insertions(+), 13 deletions(-) create mode 100644 internal/qualitygate/deptest/layering-edges.txt create mode 100644 internal/qualitygate/deptest/layering_test.go diff --git a/.github/workflows/arch-audit.yml b/.github/workflows/arch-audit.yml index 3db50f6057..691dca4964 100644 --- a/.github/workflows/arch-audit.yml +++ b/.github/workflows/arch-audit.yml @@ -62,19 +62,6 @@ jobs: go list -m -u all 2>/dev/null | grep '\[' >> report.md || echo "All dependencies up to date" >> report.md echo '```' >> report.md - - name: Circular dependency check - run: | - echo "## Circular Dependencies" >> report.md - go list -f '{{.ImportPath}} {{join .Imports " "}}' ./... | \ - go run golang.org/x/tools/cmd/digraph@v0.31.0 scc 2>&1 | tee cycles.txt - if [ -s cycles.txt ]; then - echo '```' >> report.md - cat cycles.txt >> report.md - echo '```' >> report.md - else - echo "No circular dependencies detected." >> report.md - fi - - name: E2E coverage gaps run: | echo "## E2E Coverage Gaps" >> report.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d79d3d182..d4c4382109 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -119,6 +119,20 @@ jobs: env: QUALITY_GATE_CHANGED_FROM: ${{ github.event.pull_request.base.sha || github.event.before || 'origin/main' }} run: echo "QUALITY_GATE_CHANGED_FROM=$(bash scripts/resolve-changed-from.sh)" >> "$GITHUB_ENV" + - name: Enforce layering ratchet + run: | + ratchet_file=internal/qualitygate/deptest/layering-edges.txt + initial_count=39 + if git cat-file -e "$QUALITY_GATE_CHANGED_FROM:$ratchet_file" 2>/dev/null; then + base_count=$(git show "$QUALITY_GATE_CHANGED_FROM:$ratchet_file" | grep -vcE '^\s*(#|$)' || true) + else + base_count=$initial_count + fi + current_count=$(grep -vcE '^\s*(#|$)' "$ratchet_file" || true) + if [ "$current_count" -gt "$base_count" ]; then + echo "::error::Layering ratchet grew from $base_count to $current_count effective rows. Fix the dependency instead of adding a row." + exit 1 + fi - name: Run golangci-lint run: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev="$QUALITY_GATE_CHANGED_FROM" - name: Run source-contract lint guards (lintcheck) diff --git a/internal/qualitygate/deptest/layering-edges.txt b/internal/qualitygate/deptest/layering-edges.txt new file mode 100644 index 0000000000..dae04f1df0 --- /dev/null +++ b/internal/qualitygate/deptest/layering-edges.txt @@ -0,0 +1,40 @@ +# from denied owner reason added_at +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/charcheck arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/core arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/i18n arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/keychain arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/validate arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/vfs arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/vfs/localfileio arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/charcheck arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/core arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/i18n arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/keychain arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/validate arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/vfs arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/vfs/localfileio arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/extension/transport/sidecar github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events->shortcuts inversion via convert_lib 2026-07-24 +github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/common arch-migration events->shortcuts inversion via convert_lib 2026-07-24 +github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/common arch-migration events register aggregates events/im (transitive) 2026-07-24 +github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events register aggregates events/im (transitive) 2026-07-24 +github.com/larksuite/cli/shortcuts/im github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/mail github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/minutes github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/vc github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/apps github.com/larksuite/cli/internal/keychain arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/drive github.com/larksuite/cli/internal/credential arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/im github.com/larksuite/cli/internal/credential arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/minutes github.com/larksuite/cli/internal/credential arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/vc github.com/larksuite/cli/internal/credential arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/apps github.com/larksuite/cli/internal/client arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/drive github.com/larksuite/cli/internal/client arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/im github.com/larksuite/cli/internal/client arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/mail github.com/larksuite/cli/internal/client arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/markdown github.com/larksuite/cli/internal/client arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/task github.com/larksuite/cli/internal/client arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/apps github.com/larksuite/cli/internal/vfs arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/event github.com/larksuite/cli/internal/vfs arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 +github.com/larksuite/cli/shortcuts/mail github.com/larksuite/cli/internal/vfs arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go new file mode 100644 index 0000000000..d8dd53a714 --- /dev/null +++ b/internal/qualitygate/deptest/layering_test.go @@ -0,0 +1,508 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package deptest + +import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io" + "os/exec" + "path/filepath" + "slices" + "sort" + "strings" + "testing" + "time" + + "github.com/larksuite/cli/internal/vfs" +) + +const modulePath = "github.com/larksuite/cli" + +// Mode selects whether a rule checks direct or transitive dependencies. +type Mode int + +const ( + // Direct checks package imports. + Direct Mode = iota + // Transitive checks the complete dependency set. + Transitive +) + +// Rule defines one package-layer dependency restriction. +type Rule struct { + Name string + Mode Mode + FromPrefix string + Denied []string + ExceptFrom []string + SkipFrom []string +} + +var rules = []Rule{ + { + Name: "extension-zero-internal", + Mode: Transitive, + FromPrefix: modulePath + "/extension", + Denied: []string{modulePath + "/internal/"}, + SkipFrom: []string{"/examples/"}, + }, + { + Name: "events-no-shortcuts", + Mode: Transitive, + FromPrefix: modulePath + "/events", + Denied: []string{modulePath + "/shortcuts/"}, + }, + { + Name: "shortcuts-runtime-gate", + Mode: Direct, + FromPrefix: modulePath + "/shortcuts", + Denied: []string{ + modulePath + "/internal/auth", + modulePath + "/internal/keychain", + modulePath + "/internal/credential", + modulePath + "/internal/client", + modulePath + "/internal/vfs", + }, + ExceptFrom: []string{ + modulePath + "/shortcuts/common", + modulePath + "/shortcuts/apps/gitcred", + }, + }, + { + Name: "cmd-assembly-only", + Mode: Direct, + FromPrefix: modulePath + "/cmd", + Denied: []string{modulePath + "/shortcuts"}, + ExceptFrom: []string{ + modulePath + "/cmd", + modulePath + "/cmd/auth", + }, + }, + { + Name: "errs-leaf", + Mode: Direct, + FromPrefix: modulePath + "/errs", + Denied: []string{modulePath + "/"}, + }, + { + Name: "internal-no-upper", + Mode: Direct, + FromPrefix: modulePath + "/internal", + Denied: []string{ + modulePath + "/cmd", + modulePath + "/shortcuts", + modulePath + "/events", + }, + ExceptFrom: []string{ + modulePath + "/internal/qualitygate/cmd/manifest-export", + }, + }, +} + +type listedPackage struct { + ImportPath string + Imports []string + Deps []string +} + +type layeringEdge struct { + From string + Denied string +} + +type layeringViolation struct { + layeringEdge + Rule string +} + +type seededLayeringEdge struct { + layeringEdge + Owner string + Reason string + AddedAt time.Time + Line int +} + +func TestPackageLayering(t *testing.T) { + root := repoRoot(t) + packages := goListPackageGraph(t, root) + seeded := readLayeringEdges(t, filepath.Join(root, "internal/qualitygate/deptest/layering-edges.txt")) + seededByEdge := indexSeededLayeringEdges(t, seeded) + + actualByRule := make(map[string][]layeringViolation, len(rules)) + actualEdges := make(map[layeringEdge]struct{}) + for _, rule := range rules { + violations := evaluateLayeringRule(packages, rule) + actualByRule[rule.Name] = violations + for _, violation := range violations { + actualEdges[violation.layeringEdge] = struct{}{} + } + } + + for _, rule := range rules { + t.Run(rule.Name, func(t *testing.T) { + for _, violation := range actualByRule[rule.Name] { + if _, ok := seededByEdge[violation.layeringEdge]; ok { + continue + } + t.Errorf( + "new layering violation: from=%s denied=%s rule=%s; use the approved dependency gate or fix the dependency; do not add rows to layering-edges.txt", + violation.From, + violation.Denied, + violation.Rule, + ) + } + }) + } + + t.Run("stale-layering-edges", func(t *testing.T) { + for _, edge := range seeded { + if _, ok := actualEdges[edge.layeringEdge]; ok { + continue + } + t.Errorf( + "stale layering edge: from=%s denied=%s line=%d; this violation has been removed; delete this row from layering-edges.txt", + edge.From, + edge.Denied, + edge.Line, + ) + } + }) +} + +func TestParseLayeringEdges(t *testing.T) { + t.Run("valid-rows", func(t *testing.T) { + input := strings.NewReader( + "# from\tdenied\towner\treason\tadded_at\n" + + "\n" + + "example.com/from\texample.com/denied\towner\tlegacy dependency\t2026-07-24\r\n", + ) + edges, err := parseLayeringEdges(input) + if err != nil { + t.Fatalf("parseLayeringEdges returned an error: %v", err) + } + if len(edges) != 1 { + t.Fatalf("parseLayeringEdges returned %d rows, want 1", len(edges)) + } + edge := edges[0] + if edge.From != "example.com/from" || edge.Denied != "example.com/denied" { + t.Fatalf("parseLayeringEdges returned unexpected edge: %+v", edge) + } + if edge.Owner != "owner" || edge.Reason != "legacy dependency" { + t.Fatalf("parseLayeringEdges returned unexpected metadata: %+v", edge) + } + if got := edge.AddedAt.Format(time.DateOnly); got != "2026-07-24" { + t.Fatalf("parseLayeringEdges returned added_at %q, want 2026-07-24", got) + } + if edge.Line != 3 { + t.Fatalf("parseLayeringEdges returned line %d, want 3", edge.Line) + } + }) + + tests := []struct { + name string + input string + }{ + { + name: "wrong-field-count", + input: "from\tdenied\towner\treason\n", + }, + { + name: "blank-field", + input: "from\tdenied\t\treason\t2026-07-24\n", + }, + { + name: "invalid-date", + input: "from\tdenied\towner\treason\t2026-02-30\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := parseLayeringEdges(strings.NewReader(tt.input)) + if err == nil { + t.Fatal("parseLayeringEdges returned nil error for a malformed row") + } + if !strings.Contains(err.Error(), "line 1") { + t.Fatalf("parseLayeringEdges error %q does not identify line 1", err) + } + }) + } +} + +func TestMatchesPackagePrefix(t *testing.T) { + tests := []struct { + name string + prefix string + importPath string + want bool + }{ + { + name: "exact-package", + prefix: modulePath + "/errs", + importPath: modulePath + "/errs", + want: true, + }, + { + name: "child-package", + prefix: modulePath + "/internal/vfs", + importPath: modulePath + "/internal/vfs/localfileio", + want: true, + }, + { + name: "trailing-slash-prefix", + prefix: modulePath + "/internal/", + importPath: modulePath + "/internal/vfs", + want: true, + }, + { + name: "adjacent-package-name", + prefix: modulePath + "/errs", + importPath: modulePath + "/errclass", + want: false, + }, + { + name: "unrelated-package", + prefix: modulePath + "/events/", + importPath: modulePath + "/shortcuts/im", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := matchesPackagePrefix(tt.prefix, tt.importPath); got != tt.want { + t.Fatalf( + "matchesPackagePrefix(%q, %q) = %t, want %t", + tt.prefix, + tt.importPath, + got, + tt.want, + ) + } + }) + } +} + +func TestEvaluateLayeringRuleUsesExactExceptions(t *testing.T) { + rule := Rule{ + Name: "exact-exception", + Mode: Direct, + FromPrefix: modulePath + "/cmd", + Denied: []string{modulePath + "/shortcuts"}, + ExceptFrom: []string{modulePath + "/cmd"}, + } + packages := []listedPackage{ + { + ImportPath: modulePath + "/cmd", + Imports: []string{modulePath + "/shortcuts"}, + }, + { + ImportPath: modulePath + "/cmd/service", + Imports: []string{modulePath + "/shortcuts"}, + }, + } + + violations := evaluateLayeringRule(packages, rule) + if len(violations) != 1 { + t.Fatalf("evaluateLayeringRule returned %d violations, want 1", len(violations)) + } + if got := violations[0].From; got != modulePath+"/cmd/service" { + t.Fatalf("evaluateLayeringRule returned source %q, want %q", got, modulePath+"/cmd/service") + } +} + +func goListPackageGraph(t *testing.T, root string) []listedPackage { + t.Helper() + args := []string{"list", "-json", "-tags", "authsidecar", "./..."} + cmd := exec.Command("go", args...) + cmd.Dir = root + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("go %s failed: %v\n%s", strings.Join(args, " "), err, out) + } + + decoder := json.NewDecoder(bytes.NewReader(out)) + var packages []listedPackage + for { + var pkg listedPackage + err := decoder.Decode(&pkg) + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("decode go list output: %v", err) + } + packages = append(packages, pkg) + } + return packages +} + +func evaluateLayeringRule(packages []listedPackage, rule Rule) []layeringViolation { + var violations []layeringViolation + for _, pkg := range packages { + if !matchesPackagePrefix(rule.FromPrefix, pkg.ImportPath) { + continue + } + if slices.Contains(rule.ExceptFrom, pkg.ImportPath) { + continue + } + if containsAny(pkg.ImportPath, rule.SkipFrom) { + continue + } + + dependencies := pkg.Imports + if rule.Mode == Transitive { + dependencies = pkg.Deps + } + for _, dependency := range dependencies { + if !matchesAnyPackagePrefix(rule.Denied, dependency) { + continue + } + violations = append(violations, layeringViolation{ + layeringEdge: layeringEdge{ + From: pkg.ImportPath, + Denied: dependency, + }, + Rule: rule.Name, + }) + } + } + sort.Slice(violations, func(i, j int) bool { + if violations[i].From != violations[j].From { + return violations[i].From < violations[j].From + } + return violations[i].Denied < violations[j].Denied + }) + return violations +} + +func matchesPackagePrefix(prefix, importPath string) bool { + if importPath == prefix { + return true + } + if !strings.HasPrefix(importPath, prefix) { + return false + } + return strings.HasSuffix(prefix, "/") || importPath[len(prefix)] == '/' +} + +func matchesAnyPackagePrefix(prefixes []string, importPath string) bool { + for _, prefix := range prefixes { + if matchesPackagePrefix(prefix, importPath) { + return true + } + } + return false +} + +func containsAny(value string, substrings []string) bool { + for _, substring := range substrings { + if strings.Contains(value, substring) { + return true + } + } + return false +} + +func readLayeringEdges(t *testing.T, path string) []seededLayeringEdge { + t.Helper() + file, err := vfs.Open(path) + if err != nil { + t.Fatalf("open layering edges: %v", err) + } + defer func() { + if err := file.Close(); err != nil { + t.Errorf("close layering edges: %v", err) + } + }() + + edges, err := parseLayeringEdges(file) + if err != nil { + t.Fatalf("parse layering edges: %v", err) + } + return edges +} + +func parseLayeringEdges(r io.Reader) ([]seededLayeringEdge, error) { + scanner := bufio.NewScanner(r) + var edges []seededLayeringEdge + var problems []string + for line := 1; scanner.Scan(); line++ { + text := strings.TrimRight(scanner.Text(), "\r") + if skipLayeringEdgeLine(text) { + continue + } + parts := strings.Split(text, "\t") + if len(parts) != 5 { + problems = append(problems, malformedLayeringEdge(line)) + continue + } + for i := range parts { + parts[i] = strings.TrimSpace(parts[i]) + } + addedAt, dateErr := time.Parse(time.DateOnly, parts[4]) + if hasBlank(parts...) || dateErr != nil { + problems = append(problems, malformedLayeringEdge(line)) + continue + } + edges = append(edges, seededLayeringEdge{ + layeringEdge: layeringEdge{ + From: parts[0], + Denied: parts[1], + }, + Owner: parts[2], + Reason: parts[3], + AddedAt: addedAt, + Line: line, + }) + } + if err := scanner.Err(); err != nil { + problems = append(problems, "failed to scan layering edges: "+err.Error()) + } + if len(problems) > 0 { + return nil, fmt.Errorf("%s", strings.Join(problems, "\n")) + } + return edges, nil +} + +func skipLayeringEdgeLine(text string) bool { + trimmed := strings.TrimSpace(text) + return trimmed == "" || strings.HasPrefix(trimmed, "#") +} + +func hasBlank(values ...string) bool { + for _, value := range values { + if strings.TrimSpace(value) == "" { + return true + } + } + return false +} + +func malformedLayeringEdge(line int) string { + return fmt.Sprintf( + "line %d: layering edge row must have five tab-separated non-empty fields with added_at in YYYY-MM-DD format", + line, + ) +} + +func indexSeededLayeringEdges(t *testing.T, edges []seededLayeringEdge) map[layeringEdge]seededLayeringEdge { + t.Helper() + indexed := make(map[layeringEdge]seededLayeringEdge, len(edges)) + for _, edge := range edges { + if previous, ok := indexed[edge.layeringEdge]; ok { + t.Fatalf( + "duplicate layering edge at lines %d and %d: from=%s denied=%s", + previous.Line, + edge.Line, + edge.From, + edge.Denied, + ) + } + indexed[edge.layeringEdge] = edge + } + return indexed +} From acd50f25fa9a4db67a4f0c81217cba3a4120559f Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 15:41:51 +0800 Subject: [PATCH 02/50] fix(qualitygate): harden layering ratchet enforcement --- .github/workflows/ci.yml | 14 +- Makefile | 1 + .../qualitygate/deptest/layering-edges.txt | 2 - internal/qualitygate/deptest/layering_test.go | 393 +++++++++++++++++- scripts/check-layering-ratchet.sh | 118 ++++++ scripts/check-layering-ratchet.test.sh | 220 ++++++++++ scripts/ci-workflow.test.sh | 15 + 7 files changed, 731 insertions(+), 32 deletions(-) create mode 100644 scripts/check-layering-ratchet.sh create mode 100644 scripts/check-layering-ratchet.test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d4c4382109..a9856b4011 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,19 +120,7 @@ jobs: QUALITY_GATE_CHANGED_FROM: ${{ github.event.pull_request.base.sha || github.event.before || 'origin/main' }} run: echo "QUALITY_GATE_CHANGED_FROM=$(bash scripts/resolve-changed-from.sh)" >> "$GITHUB_ENV" - name: Enforce layering ratchet - run: | - ratchet_file=internal/qualitygate/deptest/layering-edges.txt - initial_count=39 - if git cat-file -e "$QUALITY_GATE_CHANGED_FROM:$ratchet_file" 2>/dev/null; then - base_count=$(git show "$QUALITY_GATE_CHANGED_FROM:$ratchet_file" | grep -vcE '^\s*(#|$)' || true) - else - base_count=$initial_count - fi - current_count=$(grep -vcE '^\s*(#|$)' "$ratchet_file" || true) - if [ "$current_count" -gt "$base_count" ]; then - echo "::error::Layering ratchet grew from $base_count to $current_count effective rows. Fix the dependency instead of adding a row." - exit 1 - fi + run: bash scripts/check-layering-ratchet.sh "$QUALITY_GATE_CHANGED_FROM" - name: Run golangci-lint run: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev="$QUALITY_GATE_CHANGED_FROM" - name: Run source-contract lint guards (lintcheck) diff --git a/Makefile b/Makefile index a338519e3f..34888078b2 100644 --- a/Makefile +++ b/Makefile @@ -49,6 +49,7 @@ fmt-check: script-test: bash scripts/resolve-changed-from.test.sh + bash scripts/check-layering-ratchet.test.sh bash scripts/ci-workflow.test.sh bash scripts/semantic-review-workflow.test.sh $(NODE) --test scripts/e2e_domains.test.js scripts/fetch_e2e_tat.test.js scripts/install.test.js scripts/release-preflight.test.js scripts/semantic-review-verify-artifact.test.js scripts/pr-quality-summary.test.js scripts/semantic-review-publish.test.js scripts/ci-quality-summary-publish.test.js diff --git a/internal/qualitygate/deptest/layering-edges.txt b/internal/qualitygate/deptest/layering-edges.txt index dae04f1df0..10c8e36508 100644 --- a/internal/qualitygate/deptest/layering-edges.txt +++ b/internal/qualitygate/deptest/layering-edges.txt @@ -18,8 +18,6 @@ github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/i github.com/larksuite/cli/extension/transport/sidecar github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events->shortcuts inversion via convert_lib 2026-07-24 github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/common arch-migration events->shortcuts inversion via convert_lib 2026-07-24 -github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/common arch-migration events register aggregates events/im (transitive) 2026-07-24 -github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events register aggregates events/im (transitive) 2026-07-24 github.com/larksuite/cli/shortcuts/im github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 github.com/larksuite/cli/shortcuts/mail github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 github.com/larksuite/cli/shortcuts/minutes github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index d8dd53a714..5052ed3253 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -9,8 +9,10 @@ import ( "encoding/json" "fmt" "io" + "os" "os/exec" "path/filepath" + "reflect" "slices" "sort" "strings" @@ -46,20 +48,20 @@ var rules = []Rule{ { Name: "extension-zero-internal", Mode: Transitive, - FromPrefix: modulePath + "/extension", + FromPrefix: modulePath + "/extension/", Denied: []string{modulePath + "/internal/"}, SkipFrom: []string{"/examples/"}, }, { Name: "events-no-shortcuts", Mode: Transitive, - FromPrefix: modulePath + "/events", + FromPrefix: modulePath + "/events/", Denied: []string{modulePath + "/shortcuts/"}, }, { Name: "shortcuts-runtime-gate", Mode: Direct, - FromPrefix: modulePath + "/shortcuts", + FromPrefix: modulePath + "/shortcuts/", Denied: []string{ modulePath + "/internal/auth", modulePath + "/internal/keychain", @@ -91,7 +93,7 @@ var rules = []Rule{ { Name: "internal-no-upper", Mode: Direct, - FromPrefix: modulePath + "/internal", + FromPrefix: modulePath + "/internal/", Denied: []string{ modulePath + "/cmd", modulePath + "/shortcuts", @@ -109,6 +111,17 @@ type listedPackage struct { Deps []string } +type goListTarget struct { + GOOS string + GOARCH string +} + +var layeringBuildTargets = []goListTarget{ + {GOOS: "linux", GOARCH: "amd64"}, + {GOOS: "darwin", GOARCH: "amd64"}, + {GOOS: "windows", GOARCH: "amd64"}, +} + type layeringEdge struct { From string Denied string @@ -145,10 +158,7 @@ func TestPackageLayering(t *testing.T) { for _, rule := range rules { t.Run(rule.Name, func(t *testing.T) { - for _, violation := range actualByRule[rule.Name] { - if _, ok := seededByEdge[violation.layeringEdge]; ok { - continue - } + for _, violation := range findUnseededLayeringViolations(actualByRule[rule.Name], seededByEdge) { t.Errorf( "new layering violation: from=%s denied=%s rule=%s; use the approved dependency gate or fix the dependency; do not add rows to layering-edges.txt", violation.From, @@ -160,10 +170,7 @@ func TestPackageLayering(t *testing.T) { } t.Run("stale-layering-edges", func(t *testing.T) { - for _, edge := range seeded { - if _, ok := actualEdges[edge.layeringEdge]; ok { - continue - } + for _, edge := range findStaleLayeringEdges(seeded, actualEdges) { t.Errorf( "stale layering edge: from=%s denied=%s line=%d; this violation has been removed; delete this row from layering-edges.txt", edge.From, @@ -174,6 +181,37 @@ func TestPackageLayering(t *testing.T) { }) } +func TestLayeringEdgeClassification(t *testing.T) { + known := layeringEdge{From: "example.com/from", Denied: "example.com/denied"} + added := layeringEdge{From: "example.com/new", Denied: "example.com/upper"} + removed := layeringEdge{From: "example.com/old", Denied: "example.com/legacy"} + seeded := []seededLayeringEdge{ + {layeringEdge: known, Line: 1}, + {layeringEdge: removed, Line: 2}, + } + seededByEdge := map[layeringEdge]seededLayeringEdge{ + known: seeded[0], + removed: seeded[1], + } + actual := []layeringViolation{ + {layeringEdge: known, Rule: "rule"}, + {layeringEdge: added, Rule: "rule"}, + } + actualEdges := map[layeringEdge]struct{}{ + known: {}, + added: {}, + } + + unseeded := findUnseededLayeringViolations(actual, seededByEdge) + if len(unseeded) != 1 || unseeded[0].layeringEdge != added { + t.Fatalf("findUnseededLayeringViolations returned %+v, want only %+v", unseeded, added) + } + stale := findStaleLayeringEdges(seeded, actualEdges) + if len(stale) != 1 || stale[0].layeringEdge != removed { + t.Fatalf("findStaleLayeringEdges returned %+v, want only %+v", stale, removed) + } +} + func TestParseLayeringEdges(t *testing.T) { t.Run("valid-rows", func(t *testing.T) { input := strings.NewReader( @@ -314,30 +352,325 @@ func TestEvaluateLayeringRuleUsesExactExceptions(t *testing.T) { } } +func TestLayeringRuleContracts(t *testing.T) { + wantRules := []Rule{ + { + Name: "extension-zero-internal", + Mode: Transitive, + FromPrefix: modulePath + "/extension/", + Denied: []string{modulePath + "/internal/"}, + SkipFrom: []string{"/examples/"}, + }, + { + Name: "events-no-shortcuts", + Mode: Transitive, + FromPrefix: modulePath + "/events/", + Denied: []string{modulePath + "/shortcuts/"}, + }, + { + Name: "shortcuts-runtime-gate", + Mode: Direct, + FromPrefix: modulePath + "/shortcuts/", + Denied: []string{ + modulePath + "/internal/auth", + modulePath + "/internal/keychain", + modulePath + "/internal/credential", + modulePath + "/internal/client", + modulePath + "/internal/vfs", + }, + ExceptFrom: []string{ + modulePath + "/shortcuts/common", + modulePath + "/shortcuts/apps/gitcred", + }, + }, + { + Name: "cmd-assembly-only", + Mode: Direct, + FromPrefix: modulePath + "/cmd", + Denied: []string{modulePath + "/shortcuts"}, + ExceptFrom: []string{ + modulePath + "/cmd", + modulePath + "/cmd/auth", + }, + }, + { + Name: "errs-leaf", + Mode: Direct, + FromPrefix: modulePath + "/errs", + Denied: []string{modulePath + "/"}, + }, + { + Name: "internal-no-upper", + Mode: Direct, + FromPrefix: modulePath + "/internal/", + Denied: []string{ + modulePath + "/cmd", + modulePath + "/shortcuts", + modulePath + "/events", + }, + ExceptFrom: []string{ + modulePath + "/internal/qualitygate/cmd/manifest-export", + }, + }, + } + if !reflect.DeepEqual(rules, wantRules) { + t.Fatalf("layering rules differ from the enforced contract:\ngot: %#v\nwant: %#v", rules, wantRules) + } + + tests := []struct { + name string + ruleName string + packages []listedPackage + wantFrom string + wantDenied string + }{ + { + name: "extension-transitive-denial-and-example-scope-out", + ruleName: "extension-zero-internal", + packages: []listedPackage{ + { + ImportPath: modulePath + "/extension/sdk", + Deps: []string{modulePath + "/internal/core"}, + }, + { + ImportPath: modulePath + "/extension/platform/examples/demo", + Deps: []string{modulePath + "/internal/core"}, + }, + }, + wantFrom: modulePath + "/extension/sdk", + wantDenied: modulePath + "/internal/core", + }, + { + name: "events-transitive-denial", + ruleName: "events-no-shortcuts", + packages: []listedPackage{ + { + ImportPath: modulePath + "/events/im", + Deps: []string{modulePath + "/shortcuts/common"}, + }, + { + ImportPath: modulePath + "/events/calendar", + Deps: []string{modulePath + "/internal/core"}, + }, + }, + wantFrom: modulePath + "/events/im", + wantDenied: modulePath + "/shortcuts/common", + }, + { + name: "shortcuts-direct-denial-and-exceptions", + ruleName: "shortcuts-runtime-gate", + packages: []listedPackage{ + { + ImportPath: modulePath + "/shortcuts/im", + Imports: []string{modulePath + "/internal/auth"}, + }, + { + ImportPath: modulePath + "/shortcuts/common", + Imports: []string{modulePath + "/internal/auth"}, + }, + { + ImportPath: modulePath + "/shortcuts/apps/gitcred", + Imports: []string{modulePath + "/internal/keychain"}, + }, + }, + wantFrom: modulePath + "/shortcuts/im", + wantDenied: modulePath + "/internal/auth", + }, + { + name: "cmd-direct-denial-and-assembly-exceptions", + ruleName: "cmd-assembly-only", + packages: []listedPackage{ + { + ImportPath: modulePath + "/cmd/service", + Imports: []string{modulePath + "/shortcuts/im"}, + }, + { + ImportPath: modulePath + "/cmd", + Imports: []string{modulePath + "/shortcuts"}, + }, + { + ImportPath: modulePath + "/cmd/auth", + Imports: []string{modulePath + "/shortcuts/auth"}, + }, + }, + wantFrom: modulePath + "/cmd/service", + wantDenied: modulePath + "/shortcuts/im", + }, + { + name: "errs-direct-denial", + ruleName: "errs-leaf", + packages: []listedPackage{ + { + ImportPath: modulePath + "/errs", + Imports: []string{modulePath + "/internal/core"}, + }, + { + ImportPath: modulePath + "/errclass", + Imports: []string{modulePath + "/internal/core"}, + }, + }, + wantFrom: modulePath + "/errs", + wantDenied: modulePath + "/internal/core", + }, + { + name: "internal-direct-denial-and-collector-exception", + ruleName: "internal-no-upper", + packages: []listedPackage{ + { + ImportPath: modulePath + "/internal/core", + Imports: []string{modulePath + "/events/im"}, + }, + { + ImportPath: modulePath + "/internal/qualitygate/cmd/manifest-export", + Imports: []string{modulePath + "/cmd"}, + }, + }, + wantFrom: modulePath + "/internal/core", + wantDenied: modulePath + "/events/im", + }, + } + + rulesByName := make(map[string]Rule, len(rules)) + for _, rule := range rules { + rulesByName[rule.Name] = rule + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rule, ok := rulesByName[tt.ruleName] + if !ok { + t.Fatalf("missing rule %q", tt.ruleName) + } + violations := evaluateLayeringRule(tt.packages, rule) + if len(violations) != 1 { + t.Fatalf("evaluateLayeringRule returned %d violations, want 1: %+v", len(violations), violations) + } + if violations[0].From != tt.wantFrom || violations[0].Denied != tt.wantDenied { + t.Fatalf( + "evaluateLayeringRule returned edge (%q, %q), want (%q, %q)", + violations[0].From, + violations[0].Denied, + tt.wantFrom, + tt.wantDenied, + ) + } + }) + } +} + +func TestLayeringBuildTargets(t *testing.T) { + want := []goListTarget{ + {GOOS: "linux", GOARCH: "amd64"}, + {GOOS: "darwin", GOARCH: "amd64"}, + {GOOS: "windows", GOARCH: "amd64"}, + } + if !reflect.DeepEqual(layeringBuildTargets, want) { + t.Fatalf("layering build targets = %#v, want %#v", layeringBuildTargets, want) + } +} + +func TestDecodeAndMergeListedPackages(t *testing.T) { + input := strings.NewReader( + `{"ImportPath":"example.com/a","Imports":["example.com/b"],"Deps":["example.com/c"]}` + "\n" + + `{"ImportPath":"example.com/d","Imports":[],"Deps":[]}` + "\n", + ) + packages, err := decodeListedPackages(input) + if err != nil { + t.Fatalf("decodeListedPackages returned an error: %v", err) + } + if len(packages) != 2 || packages[0].ImportPath != "example.com/a" || packages[1].ImportPath != "example.com/d" { + t.Fatalf("decodeListedPackages returned unexpected packages: %+v", packages) + } + + got := mergeStrings([]string{"example.com/b", "example.com/c"}, []string{"example.com/a", "example.com/b"}) + want := []string{"example.com/a", "example.com/b", "example.com/c"} + if !slices.Equal(got, want) { + t.Fatalf("mergeStrings returned %q, want %q", got, want) + } +} + func goListPackageGraph(t *testing.T, root string) []listedPackage { + t.Helper() + packagesByPath := make(map[string]listedPackage) + for _, target := range layeringBuildTargets { + for _, pkg := range goListPackages(t, root, target) { + merged := packagesByPath[pkg.ImportPath] + merged.ImportPath = pkg.ImportPath + merged.Imports = mergeStrings(merged.Imports, pkg.Imports) + merged.Deps = mergeStrings(merged.Deps, pkg.Deps) + packagesByPath[pkg.ImportPath] = merged + } + } + + packages := make([]listedPackage, 0, len(packagesByPath)) + for _, pkg := range packagesByPath { + packages = append(packages, pkg) + } + sort.Slice(packages, func(i, j int) bool { + return packages[i].ImportPath < packages[j].ImportPath + }) + return packages +} + +func goListPackages(t *testing.T, root string, target goListTarget) []listedPackage { t.Helper() args := []string{"list", "-json", "-tags", "authsidecar", "./..."} cmd := exec.Command("go", args...) cmd.Dir = root + cmd.Env = append( + os.Environ(), + "GOOS="+target.GOOS, + "GOARCH="+target.GOARCH, + "CGO_ENABLED=0", + ) out, err := cmd.CombinedOutput() if err != nil { - t.Fatalf("go %s failed: %v\n%s", strings.Join(args, " "), err, out) + t.Fatalf( + "GOOS=%s GOARCH=%s go %s failed: %v\n%s", + target.GOOS, + target.GOARCH, + strings.Join(args, " "), + err, + out, + ) + } + + packages, err := decodeListedPackages(bytes.NewReader(out)) + if err != nil { + t.Fatalf("decode GOOS=%s GOARCH=%s go list output: %v", target.GOOS, target.GOARCH, err) } + return packages +} - decoder := json.NewDecoder(bytes.NewReader(out)) +func decodeListedPackages(r io.Reader) ([]listedPackage, error) { + decoder := json.NewDecoder(r) var packages []listedPackage for { var pkg listedPackage err := decoder.Decode(&pkg) if err == io.EOF { - break + return packages, nil } if err != nil { - t.Fatalf("decode go list output: %v", err) + return nil, err } packages = append(packages, pkg) } - return packages +} + +func mergeStrings(left, right []string) []string { + values := make(map[string]struct{}, len(left)+len(right)) + for _, value := range left { + values[value] = struct{}{} + } + for _, value := range right { + values[value] = struct{}{} + } + merged := make([]string, 0, len(values)) + for value := range values { + merged = append(merged, value) + } + sort.Strings(merged) + return merged } func evaluateLayeringRule(packages []listedPackage, rule Rule) []layeringViolation { @@ -407,6 +740,32 @@ func containsAny(value string, substrings []string) bool { return false } +func findUnseededLayeringViolations( + actual []layeringViolation, + seeded map[layeringEdge]seededLayeringEdge, +) []layeringViolation { + var unseeded []layeringViolation + for _, violation := range actual { + if _, ok := seeded[violation.layeringEdge]; !ok { + unseeded = append(unseeded, violation) + } + } + return unseeded +} + +func findStaleLayeringEdges( + seeded []seededLayeringEdge, + actual map[layeringEdge]struct{}, +) []seededLayeringEdge { + var stale []seededLayeringEdge + for _, edge := range seeded { + if _, ok := actual[edge.layeringEdge]; !ok { + stale = append(stale, edge) + } + } + return stale +} + func readLayeringEdges(t *testing.T, path string) []seededLayeringEdge { t.Helper() file, err := vfs.Open(path) diff --git a/scripts/check-layering-ratchet.sh b/scripts/check-layering-ratchet.sh new file mode 100644 index 0000000000..c8c28c5257 --- /dev/null +++ b/scripts/check-layering-ratchet.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# Copyright (c) 2026 Lark Technologies Pte. Ltd. +# SPDX-License-Identifier: MIT + +set -euo pipefail + +if root="$(git rev-parse --show-toplevel 2>/dev/null)"; then + cd "$root" +else + echo "Layering ratchet must run inside a Git worktree." >&2 + exit 1 +fi + +base_revision="${1:-${QUALITY_GATE_CHANGED_FROM:-}}" +ratchet_file="internal/qualitygate/deptest/layering-edges.txt" +initial_count="${LAYERING_RATCHET_INITIAL_COUNT:-37}" +initial_hash="${LAYERING_RATCHET_INITIAL_HASH:-cf61e4149cb4d539a1430f4a4266b91f972c253cedd300830c94e35dda1f0265}" + +if [[ -z "$base_revision" ]]; then + echo "Layering ratchet requires a base revision." >&2 + exit 1 +fi +if ! git cat-file -e "$base_revision^{commit}" 2>/dev/null; then + echo "Layering ratchet base revision does not exist: $base_revision" >&2 + exit 1 +fi +if [[ ! -f "$ratchet_file" ]]; then + echo "Layering ratchet file is missing: $ratchet_file" >&2 + exit 1 +fi + +tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/layering-ratchet.XXXXXX")" +base_file="$tmp_dir/base.txt" +base_keys="$tmp_dir/base.keys" +current_keys="$tmp_dir/current.keys" +additions="$tmp_dir/additions.keys" + +cleanup_tmp() { + rm -f "$base_file" "$base_keys" "$current_keys" "$additions" + rmdir "$tmp_dir" +} +trap cleanup_tmp EXIT + +extract_keys() { + local source_file="$1" + local output_file="$2" + awk -F '\t' ' + function trim(value) { + sub(/^[[:space:]]+/, "", value) + sub(/[[:space:]]+$/, "", value) + return value + } + { + content = trim($0) + if (content == "" || substr(content, 1, 1) == "#") { + next + } + if (NF != 5) { + printf "Malformed layering ratchet row at %s:%d: expected five tab-separated fields.\n", FILENAME, FNR > "/dev/stderr" + exit 2 + } + from = trim($1) + denied = trim($2) + owner = trim($3) + reason = trim($4) + added_at = trim($5) + if (from == "" || denied == "" || owner == "" || reason == "" || added_at !~ /^[0-9]{4}-[0-9]{2}-[0-9]{2}$/) { + printf "Malformed layering ratchet row at %s:%d: fields must be non-empty and added_at must use YYYY-MM-DD.\n", FILENAME, FNR > "/dev/stderr" + exit 2 + } + print from "\t" denied + } + ' "$source_file" | LC_ALL=C sort >"$output_file" + + if [[ -n "$(uniq -d "$output_file")" ]]; then + echo "Layering ratchet contains duplicate (from, denied) keys: $source_file" >&2 + uniq -d "$output_file" >&2 + return 1 + fi +} + +hash_keys() { + local source_file="$1" + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$source_file" | awk '{ print $1 }' + return + fi + if command -v shasum >/dev/null 2>&1; then + shasum -a 256 "$source_file" | awk '{ print $1 }' + return + fi + echo "Layering ratchet requires sha256sum or shasum." >&2 + return 1 +} + +extract_keys "$ratchet_file" "$current_keys" + +if ! git cat-file -e "$base_revision:$ratchet_file" 2>/dev/null; then + current_count="$(wc -l <"$current_keys" | tr -d '[:space:]')" + current_hash="$(hash_keys "$current_keys")" + if [[ "$current_count" != "$initial_count" || "$current_hash" != "$initial_hash" ]]; then + echo "::error::Layering ratchet bootstrap differs from the approved $initial_count-edge snapshot." >&2 + exit 1 + fi + exit 0 +fi + +git show "$base_revision:$ratchet_file" >"$base_file" +extract_keys "$base_file" "$base_keys" +LC_ALL=C comm -13 "$base_keys" "$current_keys" >"$additions" + +if [[ -s "$additions" ]]; then + echo "::error::Layering ratchet contains new (from, denied) keys. Fix the dependency instead of adding rows." >&2 + while IFS=$'\t' read -r from denied; do + printf 'from=%s denied=%s\n' "$from" "$denied" >&2 + done <"$additions" + exit 1 +fi diff --git a/scripts/check-layering-ratchet.test.sh b/scripts/check-layering-ratchet.test.sh new file mode 100644 index 0000000000..eae54fb11c --- /dev/null +++ b/scripts/check-layering-ratchet.test.sh @@ -0,0 +1,220 @@ +#!/usr/bin/env bash +# Copyright (c) 2026 Lark Technologies Pte. Ltd. +# SPDX-License-Identifier: MIT + +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +script="$repo_root/scripts/check-layering-ratchet.sh" +ratchet_file="internal/qualitygate/deptest/layering-edges.txt" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/check-layering-ratchet-test.XXXXXX")" + +cleanup_tmp() { + rm -rf "$tmp" +} +trap cleanup_tmp EXIT + +row() { + printf '%s\t%s\towner\treason\t2026-07-24\n' "$1" "$2" +} + +git_init() { + local dir="$1" + git init -q -b main "$dir" + git -C "$dir" config user.name test + git -C "$dir" config user.email test@example.com + mkdir -p "$dir/$(dirname "$ratchet_file")" +} + +write_rows() { + local dir="$1" + shift + { + printf '# from\tdenied\towner\treason\tadded_at\n' + while (( $# > 0 )); do + row "$1" "$2" + shift 2 + done + } >"$dir/$ratchet_file" +} + +commit_ratchet() { + local dir="$1" + git -C "$dir" add "$ratchet_file" + git -C "$dir" commit -q -m "ratchet" +} + +expect_pass() { + local dir="$1" + local base="$2" + if ! (cd "$dir" && bash "$script" "$base"); then + echo "Expected layering ratchet check to pass in $dir." >&2 + return 1 + fi +} + +expect_fail() { + local dir="$1" + local base="$2" + if (cd "$dir" && bash "$script" "$base" >/dev/null 2>&1); then + echo "Expected layering ratchet check to fail in $dir." >&2 + return 1 + fi +} + +hash_file() { + local source_file="$1" + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$source_file" | awk '{ print $1 }' + else + shasum -a 256 "$source_file" | awk '{ print $1 }' + fi +} + +bootstrap_keys() { + local source_file="$1" + awk -F '\t' 'NF == 5 && $1 !~ /^[[:space:]]*#/ { print $1 "\t" $2 }' "$source_file" | LC_ALL=C sort +} + +expect_bootstrap_pass() { + local dir="$1" + local base="$2" + local count="$3" + local hash="$4" + if ! ( + cd "$dir" + LAYERING_RATCHET_INITIAL_COUNT="$count" \ + LAYERING_RATCHET_INITIAL_HASH="$hash" \ + bash "$script" "$base" + ); then + echo "Expected layering ratchet bootstrap check to pass in $dir." >&2 + return 1 + fi +} + +expect_bootstrap_fail() { + local dir="$1" + local base="$2" + local count="$3" + local hash="$4" + if ( + cd "$dir" + LAYERING_RATCHET_INITIAL_COUNT="$count" \ + LAYERING_RATCHET_INITIAL_HASH="$hash" \ + bash "$script" "$base" >/dev/null 2>&1 + ); then + echo "Expected layering ratchet bootstrap check to fail in $dir." >&2 + return 1 + fi +} + +test_unchanged_and_metadata_changes_pass() { + local dir="$tmp/unchanged" + git_init "$dir" + write_rows "$dir" from/a denied/a from/b denied/b + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + expect_pass "$dir" "$base" + sed -i.bak 's/\towner\treason\t/\tnew-owner\tnew-reason\t/' "$dir/$ratchet_file" + rm -f "$dir/$ratchet_file.bak" + expect_pass "$dir" "$base" +} + +test_deletion_passes() { + local dir="$tmp/deletion" + git_init "$dir" + write_rows "$dir" from/a denied/a from/b denied/b + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + write_rows "$dir" from/a denied/a + expect_pass "$dir" "$base" +} + +test_addition_fails() { + local dir="$tmp/addition" + git_init "$dir" + write_rows "$dir" from/a denied/a + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + write_rows "$dir" from/a denied/a from/b denied/b + expect_fail "$dir" "$base" +} + +test_equal_count_replacement_fails() { + local dir="$tmp/replacement" + git_init "$dir" + write_rows "$dir" from/a denied/a from/b denied/b + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + write_rows "$dir" from/a denied/a from/c denied/c + expect_fail "$dir" "$base" +} + +test_malformed_and_missing_current_file_fail() { + local dir="$tmp/malformed" + git_init "$dir" + write_rows "$dir" from/a denied/a + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + printf 'from/a\tdenied/a\towner\treason\n' >"$dir/$ratchet_file" + expect_fail "$dir" "$base" + rm -f "$dir/$ratchet_file" + expect_fail "$dir" "$base" +} + +test_bootstrap_requires_the_approved_snapshot() { + local dir="$tmp/bootstrap" + git_init "$dir" + printf 'base\n' >"$dir/base.txt" + git -C "$dir" add base.txt + git -C "$dir" commit -q -m "base" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + local args=() + local index + for index in $(seq 1 37); do + args+=("from/$index" "denied/$index") + done + write_rows "$dir" "${args[@]}" + local keys_file="$dir/initial.keys" + bootstrap_keys "$dir/$ratchet_file" >"$keys_file" + local initial_hash + initial_hash="$(hash_file "$keys_file")" + expect_bootstrap_pass "$dir" "$base" 37 "$initial_hash" + + args[72]="from/replacement" + write_rows "$dir" "${args[@]}" + expect_bootstrap_fail "$dir" "$base" 37 "$initial_hash" + + args[72]="from/37" + args+=("from/38" "denied/38") + write_rows "$dir" "${args[@]}" + expect_bootstrap_fail "$dir" "$base" 37 "$initial_hash" +} + +test_invalid_base_fails() { + local dir="$tmp/invalid-base" + git_init "$dir" + write_rows "$dir" from/a denied/a + commit_ratchet "$dir" + expect_fail "$dir" missing-revision +} + +test_unchanged_and_metadata_changes_pass +test_deletion_passes +test_addition_fails +test_equal_count_replacement_fails +test_malformed_and_missing_current_file_fail +test_bootstrap_requires_the_approved_snapshot +test_invalid_base_fails diff --git a/scripts/ci-workflow.test.sh b/scripts/ci-workflow.test.sh index 6acdb0adc1..080376c0e2 100644 --- a/scripts/ci-workflow.test.sh +++ b/scripts/ci-workflow.test.sh @@ -170,6 +170,21 @@ if grep -Fq '${{ secrets.' <<<"$script_test_section"; then exit 1 fi +if ! grep -Fq 'bash scripts/check-layering-ratchet.sh "$QUALITY_GATE_CHANGED_FROM"' <<<"$lint_section"; then + echo "lint should enforce the layering ratchet with the tested key-set checker" + exit 1 +fi + +if grep -Fq "grep -vcE" <<<"$lint_section"; then + echo "lint should not enforce the layering ratchet by row count alone" + exit 1 +fi + +if grep -Fq "LAYERING_RATCHET_INITIAL_" <<<"$lint_section"; then + echo "lint must use the checked-in layering bootstrap snapshot" + exit 1 +fi + if grep -Fq "metadata-gate:" "$workflow"; then echo "metadata-gate should not run alongside deterministic-gate because both would upload the same facts artifact" exit 1 From 1772afe22de7666c859ec6dfae881f3a81f672d8 Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 16:01:45 +0800 Subject: [PATCH 03/50] fix(qualitygate): cover release build graphs Check all seven published GOOS and GOARCH combinations, and keep go list diagnostics separate from its JSON output for cold caches.\n\nMake the bootstrap snapshot immutable in CI, propagate shell failures explicitly, isolate sourced execution, and add deterministic regression tests for each contract. --- internal/qualitygate/deptest/layering_test.go | 84 ++++++++-- scripts/check-layering-ratchet.sh | 145 ++++++++++-------- scripts/check-layering-ratchet.test.sh | 77 +++++++--- scripts/ci-workflow.test.sh | 4 +- 4 files changed, 207 insertions(+), 103 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 5052ed3253..d2889b94ad 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -116,10 +116,16 @@ type goListTarget struct { GOARCH string } +type commandFactory func(name string, args ...string) *exec.Cmd + var layeringBuildTargets = []goListTarget{ {GOOS: "linux", GOARCH: "amd64"}, + {GOOS: "linux", GOARCH: "arm64"}, + {GOOS: "linux", GOARCH: "riscv64"}, {GOOS: "darwin", GOARCH: "amd64"}, + {GOOS: "darwin", GOARCH: "arm64"}, {GOOS: "windows", GOARCH: "amd64"}, + {GOOS: "windows", GOARCH: "arm64"}, } type layeringEdge struct { @@ -560,8 +566,12 @@ func TestLayeringRuleContracts(t *testing.T) { func TestLayeringBuildTargets(t *testing.T) { want := []goListTarget{ {GOOS: "linux", GOARCH: "amd64"}, + {GOOS: "linux", GOARCH: "arm64"}, + {GOOS: "linux", GOARCH: "riscv64"}, {GOOS: "darwin", GOARCH: "amd64"}, + {GOOS: "darwin", GOARCH: "arm64"}, {GOOS: "windows", GOARCH: "amd64"}, + {GOOS: "windows", GOARCH: "arm64"}, } if !reflect.DeepEqual(layeringBuildTargets, want) { t.Fatalf("layering build targets = %#v, want %#v", layeringBuildTargets, want) @@ -588,6 +598,33 @@ func TestDecodeAndMergeListedPackages(t *testing.T) { } } +func TestGoListPackagesSeparatesStderr(t *testing.T) { + target := goListTarget{GOOS: "linux", GOARCH: "amd64"} + packages, stderr, err := loadPackagesForTarget("", target, func(_ string, _ ...string) *exec.Cmd { + cmd := exec.Command(os.Args[0], "-test.run=^TestGoListCommandHelperProcess$") + cmd.Env = append(os.Environ(), "GO_LIST_COMMAND_HELPER=1") + return cmd + }) + if err != nil { + t.Fatalf("loadPackagesForTarget returned an error: %v", err) + } + if stderr != "go: downloading example.com/module\n" { + t.Fatalf("loadPackagesForTarget stderr = %q, want module download diagnostic", stderr) + } + if len(packages) != 1 || packages[0].ImportPath != "example.com/package" { + t.Fatalf("loadPackagesForTarget returned unexpected packages: %+v", packages) + } +} + +func TestGoListCommandHelperProcess(t *testing.T) { + if os.Getenv("GO_LIST_COMMAND_HELPER") != "1" { + return + } + fmt.Fprintln(os.Stdout, `{"ImportPath":"example.com/package","Imports":[],"Deps":[]}`) + fmt.Fprintln(os.Stderr, "go: downloading example.com/module") + os.Exit(0) +} + func goListPackageGraph(t *testing.T, root string) []listedPackage { t.Helper() packagesByPath := make(map[string]listedPackage) @@ -613,32 +650,51 @@ func goListPackageGraph(t *testing.T, root string) []listedPackage { func goListPackages(t *testing.T, root string, target goListTarget) []listedPackage { t.Helper() + packages, stderr, err := loadPackagesForTarget(root, target, exec.Command) + if err != nil { + t.Fatalf( + "GOOS=%s GOARCH=%s go list -json -tags authsidecar ./... failed: %v\n%s", + target.GOOS, + target.GOARCH, + err, + stderr, + ) + } + return packages +} + +func loadPackagesForTarget( + root string, + target goListTarget, + newCommand commandFactory, +) ([]listedPackage, string, error) { args := []string{"list", "-json", "-tags", "authsidecar", "./..."} - cmd := exec.Command("go", args...) + cmd := newCommand("go", args...) cmd.Dir = root + env := cmd.Env + if env == nil { + env = os.Environ() + } cmd.Env = append( - os.Environ(), + env, "GOOS="+target.GOOS, "GOARCH="+target.GOARCH, "CGO_ENABLED=0", ) - out, err := cmd.CombinedOutput() + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() if err != nil { - t.Fatalf( - "GOOS=%s GOARCH=%s go %s failed: %v\n%s", - target.GOOS, - target.GOARCH, - strings.Join(args, " "), - err, - out, - ) + return nil, stderr.String(), err } - packages, err := decodeListedPackages(bytes.NewReader(out)) + packages, err := decodeListedPackages(&stdout) if err != nil { - t.Fatalf("decode GOOS=%s GOARCH=%s go list output: %v", target.GOOS, target.GOARCH, err) + return nil, stderr.String(), fmt.Errorf("decode go list output: %w", err) } - return packages + return packages, stderr.String(), nil } func decodeListedPackages(r io.Reader) ([]listedPackage, error) { diff --git a/scripts/check-layering-ratchet.sh b/scripts/check-layering-ratchet.sh index c8c28c5257..9cbc82a0f7 100644 --- a/scripts/check-layering-ratchet.sh +++ b/scripts/check-layering-ratchet.sh @@ -2,46 +2,7 @@ # Copyright (c) 2026 Lark Technologies Pte. Ltd. # SPDX-License-Identifier: MIT -set -euo pipefail - -if root="$(git rev-parse --show-toplevel 2>/dev/null)"; then - cd "$root" -else - echo "Layering ratchet must run inside a Git worktree." >&2 - exit 1 -fi - -base_revision="${1:-${QUALITY_GATE_CHANGED_FROM:-}}" -ratchet_file="internal/qualitygate/deptest/layering-edges.txt" -initial_count="${LAYERING_RATCHET_INITIAL_COUNT:-37}" -initial_hash="${LAYERING_RATCHET_INITIAL_HASH:-cf61e4149cb4d539a1430f4a4266b91f972c253cedd300830c94e35dda1f0265}" - -if [[ -z "$base_revision" ]]; then - echo "Layering ratchet requires a base revision." >&2 - exit 1 -fi -if ! git cat-file -e "$base_revision^{commit}" 2>/dev/null; then - echo "Layering ratchet base revision does not exist: $base_revision" >&2 - exit 1 -fi -if [[ ! -f "$ratchet_file" ]]; then - echo "Layering ratchet file is missing: $ratchet_file" >&2 - exit 1 -fi - -tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/layering-ratchet.XXXXXX")" -base_file="$tmp_dir/base.txt" -base_keys="$tmp_dir/base.keys" -current_keys="$tmp_dir/current.keys" -additions="$tmp_dir/additions.keys" - -cleanup_tmp() { - rm -f "$base_file" "$base_keys" "$current_keys" "$additions" - rmdir "$tmp_dir" -} -trap cleanup_tmp EXIT - -extract_keys() { +layering_ratchet_extract_keys() { local source_file="$1" local output_file="$2" awk -F '\t' ' @@ -70,49 +31,105 @@ extract_keys() { } print from "\t" denied } - ' "$source_file" | LC_ALL=C sort >"$output_file" + ' "$source_file" | LC_ALL=C sort >"$output_file" || return - if [[ -n "$(uniq -d "$output_file")" ]]; then + local duplicates + duplicates="$(uniq -d "$output_file")" || return + if [[ -n "$duplicates" ]]; then echo "Layering ratchet contains duplicate (from, denied) keys: $source_file" >&2 - uniq -d "$output_file" >&2 + printf '%s\n' "$duplicates" >&2 return 1 fi } -hash_keys() { +layering_ratchet_hash_keys() { local source_file="$1" if command -v sha256sum >/dev/null 2>&1; then - sha256sum "$source_file" | awk '{ print $1 }' + sha256sum "$source_file" | awk '{ print $1 }' || return return fi if command -v shasum >/dev/null 2>&1; then - shasum -a 256 "$source_file" | awk '{ print $1 }' + shasum -a 256 "$source_file" | awk '{ print $1 }' || return return fi echo "Layering ratchet requires sha256sum or shasum." >&2 return 1 } -extract_keys "$ratchet_file" "$current_keys" +layering_ratchet_validate_bootstrap_snapshot() { + local current_keys="$1" + local expected_count="$2" + local expected_hash="$3" + local current_count + local current_hash + current_count="$(wc -l <"$current_keys" | tr -d '[:space:]')" || return + current_hash="$(layering_ratchet_hash_keys "$current_keys")" || return + if [[ "$current_count" != "$expected_count" || "$current_hash" != "$expected_hash" ]]; then + echo "::error::Layering ratchet bootstrap differs from the approved $expected_count-edge snapshot." >&2 + return 1 + fi +} + +layering_ratchet_main() ( + set -euo pipefail -if ! git cat-file -e "$base_revision:$ratchet_file" 2>/dev/null; then - current_count="$(wc -l <"$current_keys" | tr -d '[:space:]')" - current_hash="$(hash_keys "$current_keys")" - if [[ "$current_count" != "$initial_count" || "$current_hash" != "$initial_hash" ]]; then - echo "::error::Layering ratchet bootstrap differs from the approved $initial_count-edge snapshot." >&2 - exit 1 + local ratchet_file="internal/qualitygate/deptest/layering-edges.txt" + if root="$(git rev-parse --show-toplevel 2>/dev/null)"; then + cd "$root" || return + else + echo "Layering ratchet must run inside a Git worktree." >&2 + return 1 fi - exit 0 -fi -git show "$base_revision:$ratchet_file" >"$base_file" -extract_keys "$base_file" "$base_keys" -LC_ALL=C comm -13 "$base_keys" "$current_keys" >"$additions" + local base_revision="${1:-${QUALITY_GATE_CHANGED_FROM:-}}" + local approved_initial_count="${2:-37}" + local approved_initial_hash="${3:-cf61e4149cb4d539a1430f4a4266b91f972c253cedd300830c94e35dda1f0265}" + if [[ -z "$base_revision" ]]; then + echo "Layering ratchet requires a base revision." >&2 + return 1 + fi + if ! git cat-file -e "$base_revision^{commit}" 2>/dev/null; then + echo "Layering ratchet base revision does not exist: $base_revision" >&2 + return 1 + fi + if [[ ! -f "$ratchet_file" ]]; then + echo "Layering ratchet file is missing: $ratchet_file" >&2 + return 1 + fi + + local tmp_dir + tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/layering-ratchet.XXXXXX")" || return + local base_file="$tmp_dir/base.txt" + local base_keys="$tmp_dir/base.keys" + local current_keys="$tmp_dir/current.keys" + local additions="$tmp_dir/additions.keys" + + layering_ratchet_cleanup_current_run() { + rm -f "$base_file" "$base_keys" "$current_keys" "$additions" + rmdir "$tmp_dir" + } + trap layering_ratchet_cleanup_current_run EXIT + + layering_ratchet_extract_keys "$ratchet_file" "$current_keys" || return + + if ! git cat-file -e "$base_revision:$ratchet_file" 2>/dev/null; then + layering_ratchet_validate_bootstrap_snapshot "$current_keys" "$approved_initial_count" "$approved_initial_hash" || return + return + fi + + git show "$base_revision:$ratchet_file" >"$base_file" || return + layering_ratchet_extract_keys "$base_file" "$base_keys" || return + LC_ALL=C comm -13 "$base_keys" "$current_keys" >"$additions" || return + + if [[ -s "$additions" ]]; then + echo "::error::Layering ratchet contains new (from, denied) keys. Fix the dependency instead of adding rows." >&2 + while IFS=$'\t' read -r from denied; do + printf 'from=%s denied=%s\n' "$from" "$denied" >&2 + done <"$additions" + return 1 + fi +) -if [[ -s "$additions" ]]; then - echo "::error::Layering ratchet contains new (from, denied) keys. Fix the dependency instead of adding rows." >&2 - while IFS=$'\t' read -r from denied; do - printf 'from=%s denied=%s\n' "$from" "$denied" >&2 - done <"$additions" - exit 1 +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + layering_ratchet_main "${1:-${QUALITY_GATE_CHANGED_FROM:-}}" fi diff --git a/scripts/check-layering-ratchet.test.sh b/scripts/check-layering-ratchet.test.sh index eae54fb11c..39744b96b3 100644 --- a/scripts/check-layering-ratchet.test.sh +++ b/scripts/check-layering-ratchet.test.sh @@ -8,6 +8,7 @@ repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" script="$repo_root/scripts/check-layering-ratchet.sh" ratchet_file="internal/qualitygate/deptest/layering-edges.txt" tmp="$(mktemp -d "${TMPDIR:-/tmp}/check-layering-ratchet-test.XXXXXX")" +source "$script" cleanup_tmp() { rm -rf "$tmp" @@ -56,10 +57,16 @@ expect_pass() { expect_fail() { local dir="$1" local base="$2" - if (cd "$dir" && bash "$script" "$base" >/dev/null 2>&1); then + local expected="$3" + local output + if output="$(cd "$dir" && bash "$script" "$base" 2>&1)"; then echo "Expected layering ratchet check to fail in $dir." >&2 return 1 fi + if ! grep -Fq "$expected" <<<"$output"; then + printf 'Layering ratchet failure did not include %q:\n%s\n' "$expected" "$output" >&2 + return 1 + fi } hash_file() { @@ -81,12 +88,7 @@ expect_bootstrap_pass() { local base="$2" local count="$3" local hash="$4" - if ! ( - cd "$dir" - LAYERING_RATCHET_INITIAL_COUNT="$count" \ - LAYERING_RATCHET_INITIAL_HASH="$hash" \ - bash "$script" "$base" - ); then + if ! (cd "$dir" && layering_ratchet_main "$base" "$count" "$hash"); then echo "Expected layering ratchet bootstrap check to pass in $dir." >&2 return 1 fi @@ -97,15 +99,30 @@ expect_bootstrap_fail() { local base="$2" local count="$3" local hash="$4" - if ( - cd "$dir" - LAYERING_RATCHET_INITIAL_COUNT="$count" \ - LAYERING_RATCHET_INITIAL_HASH="$hash" \ - bash "$script" "$base" >/dev/null 2>&1 - ); then + local output + if output="$(cd "$dir" && layering_ratchet_main "$base" "$count" "$hash" 2>&1)"; then echo "Expected layering ratchet bootstrap check to fail in $dir." >&2 return 1 fi + if ! grep -Fq "bootstrap differs from the approved" <<<"$output"; then + printf 'Unexpected layering ratchet bootstrap failure:\n%s\n' "$output" >&2 + return 1 + fi +} + +expect_sourced_main_fail() { + local dir="$1" + local base="$2" + local expected="$3" + local output + if output="$(cd "$dir" && layering_ratchet_main "$base" 2>&1)"; then + echo "Expected sourced layering ratchet main to fail in $dir." >&2 + return 1 + fi + if ! grep -Fq "$expected" <<<"$output"; then + printf 'Sourced layering ratchet failure did not include %q:\n%s\n' "$expected" "$output" >&2 + return 1 + fi } test_unchanged_and_metadata_changes_pass() { @@ -143,7 +160,7 @@ test_addition_fails() { base="$(git -C "$dir" rev-parse HEAD)" write_rows "$dir" from/a denied/a from/b denied/b - expect_fail "$dir" "$base" + expect_fail "$dir" "$base" "from=from/b denied=denied/b" } test_equal_count_replacement_fails() { @@ -155,7 +172,7 @@ test_equal_count_replacement_fails() { base="$(git -C "$dir" rev-parse HEAD)" write_rows "$dir" from/a denied/a from/c denied/c - expect_fail "$dir" "$base" + expect_fail "$dir" "$base" "from=from/c denied=denied/c" } test_malformed_and_missing_current_file_fail() { @@ -167,9 +184,22 @@ test_malformed_and_missing_current_file_fail() { base="$(git -C "$dir" rev-parse HEAD)" printf 'from/a\tdenied/a\towner\treason\n' >"$dir/$ratchet_file" - expect_fail "$dir" "$base" + expect_fail "$dir" "$base" "expected five tab-separated fields" + expect_sourced_main_fail "$dir" "$base" "expected five tab-separated fields" rm -f "$dir/$ratchet_file" - expect_fail "$dir" "$base" + expect_fail "$dir" "$base" "Layering ratchet file is missing" +} + +test_duplicate_key_fails() { + local dir="$tmp/duplicate" + git_init "$dir" + write_rows "$dir" from/a denied/a + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + write_rows "$dir" from/a denied/a from/a denied/a + expect_fail "$dir" "$base" "duplicate (from, denied) keys" } test_bootstrap_requires_the_approved_snapshot() { @@ -189,18 +219,18 @@ test_bootstrap_requires_the_approved_snapshot() { write_rows "$dir" "${args[@]}" local keys_file="$dir/initial.keys" bootstrap_keys "$dir/$ratchet_file" >"$keys_file" - local initial_hash - initial_hash="$(hash_file "$keys_file")" - expect_bootstrap_pass "$dir" "$base" 37 "$initial_hash" + local expected_hash + expected_hash="$(hash_file "$keys_file")" + expect_bootstrap_pass "$dir" "$base" 37 "$expected_hash" args[72]="from/replacement" write_rows "$dir" "${args[@]}" - expect_bootstrap_fail "$dir" "$base" 37 "$initial_hash" + expect_bootstrap_fail "$dir" "$base" 37 "$expected_hash" args[72]="from/37" args+=("from/38" "denied/38") write_rows "$dir" "${args[@]}" - expect_bootstrap_fail "$dir" "$base" 37 "$initial_hash" + expect_bootstrap_fail "$dir" "$base" 37 "$expected_hash" } test_invalid_base_fails() { @@ -208,7 +238,7 @@ test_invalid_base_fails() { git_init "$dir" write_rows "$dir" from/a denied/a commit_ratchet "$dir" - expect_fail "$dir" missing-revision + expect_fail "$dir" missing-revision "base revision does not exist" } test_unchanged_and_metadata_changes_pass @@ -216,5 +246,6 @@ test_deletion_passes test_addition_fails test_equal_count_replacement_fails test_malformed_and_missing_current_file_fail +test_duplicate_key_fails test_bootstrap_requires_the_approved_snapshot test_invalid_base_fails diff --git a/scripts/ci-workflow.test.sh b/scripts/ci-workflow.test.sh index 080376c0e2..b6856f887e 100644 --- a/scripts/ci-workflow.test.sh +++ b/scripts/ci-workflow.test.sh @@ -180,8 +180,8 @@ if grep -Fq "grep -vcE" <<<"$lint_section"; then exit 1 fi -if grep -Fq "LAYERING_RATCHET_INITIAL_" <<<"$lint_section"; then - echo "lint must use the checked-in layering bootstrap snapshot" +if grep -Fq "LAYERING_RATCHET_INITIAL_" "$workflow"; then + echo "CI must use the immutable checked-in layering bootstrap snapshot" exit 1 fi From c09b0d5dd36941ad1fa273b014e0992b6e3b467b Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 16:09:25 +0800 Subject: [PATCH 04/50] test(qualitygate): tolerate coverage helper diagnostics --- internal/qualitygate/deptest/layering_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index d2889b94ad..44ac2fa0fc 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -608,7 +608,7 @@ func TestGoListPackagesSeparatesStderr(t *testing.T) { if err != nil { t.Fatalf("loadPackagesForTarget returned an error: %v", err) } - if stderr != "go: downloading example.com/module\n" { + if !strings.Contains(stderr, "go: downloading example.com/module\n") { t.Fatalf("loadPackagesForTarget stderr = %q, want module download diagnostic", stderr) } if len(packages) != 1 || packages[0].ImportPath != "example.com/package" { From f1ce88b48e9427550cca6846cbc0c40c22166ad8 Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 16:16:55 +0800 Subject: [PATCH 05/50] test(qualitygate): pin release target coverage --- internal/qualitygate/deptest/layering_test.go | 79 ++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 44ac2fa0fc..4c451d84bb 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -20,6 +20,7 @@ import ( "time" "github.com/larksuite/cli/internal/vfs" + "gopkg.in/yaml.v3" ) const modulePath = "github.com/larksuite/cli" @@ -112,12 +113,20 @@ type listedPackage struct { } type goListTarget struct { - GOOS string - GOARCH string + GOOS string `yaml:"goos"` + GOARCH string `yaml:"goarch"` } type commandFactory func(name string, args ...string) *exec.Cmd +type goReleaserConfig struct { + Builds []struct { + GOOS []string `yaml:"goos"` + GOARCH []string `yaml:"goarch"` + Ignore []goListTarget `yaml:"ignore"` + } `yaml:"builds"` +} + var layeringBuildTargets = []goListTarget{ {GOOS: "linux", GOARCH: "amd64"}, {GOOS: "linux", GOARCH: "arm64"}, @@ -578,6 +587,63 @@ func TestLayeringBuildTargets(t *testing.T) { } } +func TestLayeringBuildTargetsMatchGoReleaser(t *testing.T) { + root := repoRoot(t) + content, err := vfs.ReadFile(filepath.Join(root, ".goreleaser.yml")) + if err != nil { + t.Fatalf("read .goreleaser.yml: %v", err) + } + + var config goReleaserConfig + if err := yaml.Unmarshal(content, &config); err != nil { + t.Fatalf("parse .goreleaser.yml: %v", err) + } + if len(config.Builds) == 0 { + t.Fatal(".goreleaser.yml has no builds") + } + + output, err := exec.Command("go", "tool", "dist", "list").Output() + if err != nil { + t.Fatalf("go tool dist list: %v", err) + } + supported := make(map[string]struct{}) + for _, target := range strings.Fields(string(output)) { + supported[target] = struct{}{} + } + + want := make(map[string]struct{}) + for _, build := range config.Builds { + ignored := make(map[string]struct{}, len(build.Ignore)) + for _, target := range build.Ignore { + ignored[target.GOOS+"/"+target.GOARCH] = struct{}{} + } + for _, goos := range build.GOOS { + for _, goarch := range build.GOARCH { + target := goos + "/" + goarch + if _, ok := supported[target]; !ok { + continue + } + if _, ok := ignored[target]; ok { + continue + } + want[target] = struct{}{} + } + } + } + + got := make(map[string]struct{}, len(layeringBuildTargets)) + for _, target := range layeringBuildTargets { + key := target.GOOS + "/" + target.GOARCH + if _, duplicate := got[key]; duplicate { + t.Fatalf("layering build target %q is duplicated", key) + } + got[key] = struct{}{} + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("layering build targets = %v, want GoReleaser targets %v", sortedKeys(got), sortedKeys(want)) + } +} + func TestDecodeAndMergeListedPackages(t *testing.T) { input := strings.NewReader( `{"ImportPath":"example.com/a","Imports":["example.com/b"],"Deps":["example.com/c"]}` + "\n" + @@ -729,6 +795,15 @@ func mergeStrings(left, right []string) []string { return merged } +func sortedKeys(values map[string]struct{}) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + slices.Sort(keys) + return keys +} + func evaluateLayeringRule(packages []listedPackage, rule Rule) []layeringViolation { var violations []layeringViolation for _, pkg := range packages { From 59b6393250dcfb2eca5b5dadcc50846ad66ff6ce Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 16:24:30 +0800 Subject: [PATCH 06/50] test(qualitygate): fail closed on release target drift --- internal/qualitygate/deptest/layering_test.go | 297 ++++++++++++++++-- 1 file changed, 272 insertions(+), 25 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 4c451d84bb..1a0ceb45bd 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -13,6 +13,7 @@ import ( "os/exec" "path/filepath" "reflect" + "regexp" "slices" "sort" "strings" @@ -120,11 +121,22 @@ type goListTarget struct { type commandFactory func(name string, args ...string) *exec.Cmd type goReleaserConfig struct { - Builds []struct { - GOOS []string `yaml:"goos"` - GOARCH []string `yaml:"goarch"` - Ignore []goListTarget `yaml:"ignore"` - } `yaml:"builds"` + Builds []goReleaserBuild `yaml:"builds"` +} + +type goReleaserBuild struct { + Builder string `yaml:"builder"` + GOOS []string `yaml:"goos"` + GOARCH []string `yaml:"goarch"` + Targets []string `yaml:"targets"` + Ignore []map[string]any `yaml:"ignore"` + Skip bool `yaml:"skip"` +} + +type distTarget struct { + GOOS string + GOARCH string + FirstClass bool } var layeringBuildTargets = []goListTarget{ @@ -602,32 +614,27 @@ func TestLayeringBuildTargetsMatchGoReleaser(t *testing.T) { t.Fatal(".goreleaser.yml has no builds") } - output, err := exec.Command("go", "tool", "dist", "list").Output() + output, err := exec.Command("go", "tool", "dist", "list", "-json").Output() if err != nil { t.Fatalf("go tool dist list: %v", err) } - supported := make(map[string]struct{}) - for _, target := range strings.Fields(string(output)) { - supported[target] = struct{}{} + var distTargets []distTarget + if err := json.Unmarshal(output, &distTargets); err != nil { + t.Fatalf("parse go tool dist list: %v", err) + } + supported := make(map[string]distTarget, len(distTargets)) + for _, target := range distTargets { + supported[target.GOOS+"/"+target.GOARCH] = target } want := make(map[string]struct{}) - for _, build := range config.Builds { - ignored := make(map[string]struct{}, len(build.Ignore)) - for _, target := range build.Ignore { - ignored[target.GOOS+"/"+target.GOARCH] = struct{}{} - } - for _, goos := range build.GOOS { - for _, goarch := range build.GOARCH { - target := goos + "/" + goarch - if _, ok := supported[target]; !ok { - continue - } - if _, ok := ignored[target]; ok { - continue - } - want[target] = struct{}{} - } + for index, build := range config.Builds { + targets, err := goReleaserBuildTargets(build, supported) + if err != nil { + t.Fatalf(".goreleaser.yml build %d: %v", index, err) + } + for target := range targets { + want[target] = struct{}{} } } @@ -644,6 +651,246 @@ func TestLayeringBuildTargetsMatchGoReleaser(t *testing.T) { } } +func TestReleaseGoVersionMatchesModule(t *testing.T) { + root := repoRoot(t) + moduleContent, err := vfs.ReadFile(filepath.Join(root, "go.mod")) + if err != nil { + t.Fatalf("read go.mod: %v", err) + } + releaseContent, err := vfs.ReadFile(filepath.Join(root, ".github", "workflows", "release.yml")) + if err != nil { + t.Fatalf("read release workflow: %v", err) + } + + moduleVersion := findVersion(t, string(moduleContent), `(?m)^go\s+([0-9]+\.[0-9]+)(?:\.[0-9]+)?\s*$`, "go.mod") + releaseVersion := findVersion( + t, + string(releaseContent), + `(?m)^\s+go-version:\s*['"]?([0-9]+\.[0-9]+)(?:\.[0-9]+)?['"]?\s*$`, + "release workflow", + ) + if releaseVersion != moduleVersion { + t.Fatalf("release Go version = %q, want module Go version %q", releaseVersion, moduleVersion) + } +} + +func TestGoReleaserBuildTargets(t *testing.T) { + supported := map[string]distTarget{ + "darwin/arm64": {GOOS: "darwin", GOARCH: "arm64", FirstClass: true}, + "freebsd/amd64": {GOOS: "freebsd", GOARCH: "amd64"}, + "linux/amd64": {GOOS: "linux", GOARCH: "amd64", FirstClass: true}, + "windows/amd64": {GOOS: "windows", GOARCH: "amd64", FirstClass: true}, + "windows/arm64": {GOOS: "windows", GOARCH: "arm64"}, + } + tests := []struct { + name string + build goReleaserBuild + want []string + wantErr bool + }{ + { + name: "explicit targets override the matrix", + build: goReleaserBuild{ + GOOS: []string{"linux"}, + GOARCH: []string{"amd64"}, + Targets: []string{"freebsd_amd64"}, + }, + want: []string{"freebsd/amd64"}, + }, + { + name: "target suffixes preserve the base platform", + build: goReleaserBuild{ + Targets: []string{"linux_amd64_v1"}, + }, + want: []string{"linux/amd64"}, + }, + { + name: "first class selector expands from the toolchain", + build: goReleaserBuild{ + Targets: []string{"go_first_class"}, + }, + want: []string{"darwin/arm64", "linux/amd64", "windows/amd64"}, + }, + { + name: "partial ignore matches every architecture", + build: goReleaserBuild{ + GOOS: []string{"windows"}, + GOARCH: []string{"amd64", "arm64"}, + Ignore: []map[string]any{{"goos": "windows"}}, + }, + want: []string{}, + }, + { + name: "skipped build has no targets", + build: goReleaserBuild{ + Skip: true, + }, + want: []string{}, + }, + { + name: "microarchitecture ignore fails closed", + build: goReleaserBuild{ + GOOS: []string{"linux"}, + GOARCH: []string{"amd64"}, + Ignore: []map[string]any{{"goamd64": "v1"}}, + }, + wantErr: true, + }, + { + name: "fixed first class selector fails closed", + build: goReleaserBuild{ + Targets: []string{"go_118_first_class"}, + }, + wantErr: true, + }, + { + name: "non-Go builder fails closed", + build: goReleaserBuild{ + Builder: "rust", + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := goReleaserBuildTargets(tt.build, supported) + if tt.wantErr { + if err == nil { + t.Fatal("goReleaserBuildTargets returned no error") + } + return + } + if err != nil { + t.Fatalf("goReleaserBuildTargets returned an error: %v", err) + } + if keys := sortedKeys(got); !slices.Equal(keys, tt.want) { + t.Fatalf("goReleaserBuildTargets = %v, want %v", keys, tt.want) + } + }) + } +} + +func goReleaserBuildTargets( + build goReleaserBuild, + supported map[string]distTarget, +) (map[string]struct{}, error) { + if build.Skip { + return map[string]struct{}{}, nil + } + if build.Builder != "" && build.Builder != "go" { + return nil, fmt.Errorf("unsupported builder %q", build.Builder) + } + if len(build.Targets) > 0 { + return explicitGoReleaserTargets(build.Targets, supported) + } + + gooses := build.GOOS + if len(gooses) == 0 { + gooses = []string{"darwin", "linux", "windows"} + } + goarches := build.GOARCH + if len(goarches) == 0 { + goarches = []string{"386", "amd64", "arm64"} + } + + targets := make(map[string]struct{}) + for _, goos := range gooses { + for _, goarch := range goarches { + target := goos + "/" + goarch + if _, ok := supported[target]; !ok { + continue + } + ignored, err := goReleaserTargetIgnored(goos, goarch, build.Ignore) + if err != nil { + return nil, err + } + if !ignored { + targets[target] = struct{}{} + } + } + } + return targets, nil +} + +func explicitGoReleaserTargets( + configured []string, + supported map[string]distTarget, +) (map[string]struct{}, error) { + targets := make(map[string]struct{}) + for _, configuredTarget := range configured { + if configuredTarget == "go_first_class" { + for key, target := range supported { + if target.FirstClass { + targets[key] = struct{}{} + } + } + continue + } + if configuredTarget == "go_118_first_class" { + return nil, fmt.Errorf("unsupported target selector %q", configuredTarget) + } + if strings.Contains(configuredTarget, "{{") { + return nil, fmt.Errorf("templated target %q is unsupported", configuredTarget) + } + + parts := strings.Split(configuredTarget, "_") + if len(parts) < 2 { + return nil, fmt.Errorf("malformed target %q", configuredTarget) + } + target := parts[0] + "/" + parts[1] + if _, ok := supported[target]; !ok { + return nil, fmt.Errorf("unsupported Go target %q", configuredTarget) + } + targets[target] = struct{}{} + } + return targets, nil +} + +func goReleaserTargetIgnored(goos, goarch string, ignored []map[string]any) (bool, error) { + for _, entry := range ignored { + for key := range entry { + if key != "goos" && key != "goarch" { + return false, fmt.Errorf("unsupported ignore selector %q", key) + } + } + ignoredGOOS, err := optionalString(entry, "goos") + if err != nil { + return false, err + } + ignoredGOARCH, err := optionalString(entry, "goarch") + if err != nil { + return false, err + } + if (ignoredGOOS == "" || ignoredGOOS == goos) && + (ignoredGOARCH == "" || ignoredGOARCH == goarch) { + return true, nil + } + } + return false, nil +} + +func optionalString(values map[string]any, key string) (string, error) { + value, ok := values[key] + if !ok { + return "", nil + } + text, ok := value.(string) + if !ok { + return "", fmt.Errorf("ignore selector %q must be a string", key) + } + return text, nil +} + +func findVersion(t *testing.T, content, pattern, source string) string { + t.Helper() + match := regexp.MustCompile(pattern).FindStringSubmatch(content) + if len(match) != 2 { + t.Fatalf("find Go version in %s", source) + } + return match[1] +} + func TestDecodeAndMergeListedPackages(t *testing.T) { input := strings.NewReader( `{"ImportPath":"example.com/a","Imports":["example.com/b"],"Deps":["example.com/c"]}` + "\n" + From cbe0fb12df42f2a927533522578ae482c3717991 Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 16:29:47 +0800 Subject: [PATCH 07/50] test(qualitygate): reject unsupported release variants --- internal/qualitygate/deptest/layering_test.go | 68 ++++++++++++++++--- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 1a0ceb45bd..0c582b6596 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -125,12 +125,22 @@ type goReleaserConfig struct { } type goReleaserBuild struct { - Builder string `yaml:"builder"` - GOOS []string `yaml:"goos"` - GOARCH []string `yaml:"goarch"` - Targets []string `yaml:"targets"` - Ignore []map[string]any `yaml:"ignore"` - Skip bool `yaml:"skip"` + Builder string `yaml:"builder"` + GOOS []string `yaml:"goos"` + GOARCH []string `yaml:"goarch"` + GOARM []any `yaml:"goarm"` + GOAMD64 []any `yaml:"goamd64"` + GOARM64 []any `yaml:"goarm64"` + GOMIPS []any `yaml:"gomips"` + GOMIPS64 []any `yaml:"gomips64"` + GO386 []any `yaml:"go386"` + GOPPC64 []any `yaml:"goppc64"` + GORISCV64 []any `yaml:"goriscv64"` + Tool string `yaml:"tool"` + GoBinary string `yaml:"gobinary"` + Targets []string `yaml:"targets"` + Ignore []map[string]any `yaml:"ignore"` + Skip bool `yaml:"skip"` } type distTarget struct { @@ -698,11 +708,11 @@ func TestGoReleaserBuildTargets(t *testing.T) { want: []string{"freebsd/amd64"}, }, { - name: "target suffixes preserve the base platform", + name: "target suffixes fail closed", build: goReleaserBuild{ Targets: []string{"linux_amd64_v1"}, }, - want: []string{"linux/amd64"}, + wantErr: true, }, { name: "first class selector expands from the toolchain", @@ -727,6 +737,15 @@ func TestGoReleaserBuildTargets(t *testing.T) { }, want: []string{}, }, + { + name: "microarchitecture matrix fails closed", + build: goReleaserBuild{ + GOOS: []string{"linux"}, + GOARCH: []string{"amd64"}, + GOAMD64: []any{"v3"}, + }, + wantErr: true, + }, { name: "microarchitecture ignore fails closed", build: goReleaserBuild{ @@ -750,6 +769,20 @@ func TestGoReleaserBuildTargets(t *testing.T) { }, wantErr: true, }, + { + name: "custom Go tool fails closed", + build: goReleaserBuild{ + Tool: "go1.24.0", + }, + wantErr: true, + }, + { + name: "legacy custom Go binary fails closed", + build: goReleaserBuild{ + GoBinary: "go1.24.0", + }, + wantErr: true, + }, } for _, tt := range tests { @@ -781,6 +814,12 @@ func goReleaserBuildTargets( if build.Builder != "" && build.Builder != "go" { return nil, fmt.Errorf("unsupported builder %q", build.Builder) } + if build.Tool != "" || build.GoBinary != "" { + return nil, fmt.Errorf("custom Go tools are unsupported") + } + if buildHasMicroarchitectureMatrix(build) { + return nil, fmt.Errorf("microarchitecture matrices are unsupported") + } if len(build.Targets) > 0 { return explicitGoReleaserTargets(build.Targets, supported) } @@ -835,7 +874,7 @@ func explicitGoReleaserTargets( } parts := strings.Split(configuredTarget, "_") - if len(parts) < 2 { + if len(parts) != 2 { return nil, fmt.Errorf("malformed target %q", configuredTarget) } target := parts[0] + "/" + parts[1] @@ -847,6 +886,17 @@ func explicitGoReleaserTargets( return targets, nil } +func buildHasMicroarchitectureMatrix(build goReleaserBuild) bool { + return len(build.GOARM) > 0 || + len(build.GOAMD64) > 0 || + len(build.GOARM64) > 0 || + len(build.GOMIPS) > 0 || + len(build.GOMIPS64) > 0 || + len(build.GO386) > 0 || + len(build.GOPPC64) > 0 || + len(build.GORISCV64) > 0 +} + func goReleaserTargetIgnored(goos, goarch string, ignored []map[string]any) (bool, error) { for _, entry := range ignored { for key := range entry { From 7c2ca4e4651879a63701377f66e8df70cb8f3b5e Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 16:34:22 +0800 Subject: [PATCH 08/50] test(qualitygate): cover default release graph --- internal/qualitygate/deptest/layering_test.go | 147 ++++++++++++++---- 1 file changed, 119 insertions(+), 28 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 0c582b6596..98e2a5e6d4 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -138,15 +138,17 @@ type goReleaserBuild struct { GORISCV64 []any `yaml:"goriscv64"` Tool string `yaml:"tool"` GoBinary string `yaml:"gobinary"` + Tags []string `yaml:"tags"` + Flags []string `yaml:"flags"` + Env []string `yaml:"env"` Targets []string `yaml:"targets"` Ignore []map[string]any `yaml:"ignore"` Skip bool `yaml:"skip"` } type distTarget struct { - GOOS string - GOARCH string - FirstClass bool + GOOS string + GOARCH string } var layeringBuildTargets = []goListTarget{ @@ -159,6 +161,8 @@ var layeringBuildTargets = []goListTarget{ {GOOS: "windows", GOARCH: "arm64"}, } +var layeringBuildTags = []string{"", "authsidecar"} + type layeringEdge struct { From string Denied string @@ -609,6 +613,13 @@ func TestLayeringBuildTargets(t *testing.T) { } } +func TestLayeringBuildTags(t *testing.T) { + want := []string{"", "authsidecar"} + if !slices.Equal(layeringBuildTags, want) { + t.Fatalf("layering build tags = %q, want %q", layeringBuildTags, want) + } +} + func TestLayeringBuildTargetsMatchGoReleaser(t *testing.T) { root := repoRoot(t) content, err := vfs.ReadFile(filepath.Join(root, ".goreleaser.yml")) @@ -686,10 +697,10 @@ func TestReleaseGoVersionMatchesModule(t *testing.T) { func TestGoReleaserBuildTargets(t *testing.T) { supported := map[string]distTarget{ - "darwin/arm64": {GOOS: "darwin", GOARCH: "arm64", FirstClass: true}, + "darwin/arm64": {GOOS: "darwin", GOARCH: "arm64"}, "freebsd/amd64": {GOOS: "freebsd", GOARCH: "amd64"}, - "linux/amd64": {GOOS: "linux", GOARCH: "amd64", FirstClass: true}, - "windows/amd64": {GOOS: "windows", GOARCH: "amd64", FirstClass: true}, + "linux/amd64": {GOOS: "linux", GOARCH: "amd64"}, + "windows/amd64": {GOOS: "windows", GOARCH: "amd64"}, "windows/arm64": {GOOS: "windows", GOARCH: "arm64"}, } tests := []struct { @@ -715,11 +726,11 @@ func TestGoReleaserBuildTargets(t *testing.T) { wantErr: true, }, { - name: "first class selector expands from the toolchain", + name: "first class selector fails closed", build: goReleaserBuild{ Targets: []string{"go_first_class"}, }, - want: []string{"darwin/arm64", "linux/amd64", "windows/amd64"}, + wantErr: true, }, { name: "partial ignore matches every architecture", @@ -737,6 +748,14 @@ func TestGoReleaserBuildTargets(t *testing.T) { }, want: []string{}, }, + { + name: "arm target fails closed", + build: goReleaserBuild{ + GOOS: []string{"linux"}, + GOARCH: []string{"arm"}, + }, + wantErr: true, + }, { name: "microarchitecture matrix fails closed", build: goReleaserBuild{ @@ -783,6 +802,34 @@ func TestGoReleaserBuildTargets(t *testing.T) { }, wantErr: true, }, + { + name: "release build tags fail closed", + build: goReleaserBuild{ + Tags: []string{"feature"}, + }, + wantErr: true, + }, + { + name: "build flag tags fail closed", + build: goReleaserBuild{ + Flags: []string{"-tags=feature"}, + }, + wantErr: true, + }, + { + name: "environment build tags fail closed", + build: goReleaserBuild{ + Env: []string{"GOFLAGS=-tags=feature"}, + }, + wantErr: true, + }, + { + name: "enabled cgo fails closed", + build: goReleaserBuild{ + Env: []string{"CGO_ENABLED=1"}, + }, + wantErr: true, + }, } for _, tt := range tests { @@ -817,6 +864,12 @@ func goReleaserBuildTargets( if build.Tool != "" || build.GoBinary != "" { return nil, fmt.Errorf("custom Go tools are unsupported") } + if len(build.Tags) > 0 || buildFlagsSetTags(build.Flags) { + return nil, fmt.Errorf("release build tags are unsupported") + } + if err := validateGoReleaserBuildEnv(build.Env); err != nil { + return nil, err + } if buildHasMicroarchitectureMatrix(build) { return nil, fmt.Errorf("microarchitecture matrices are unsupported") } @@ -836,6 +889,9 @@ func goReleaserBuildTargets( targets := make(map[string]struct{}) for _, goos := range gooses { for _, goarch := range goarches { + if goarch == "arm" { + return nil, fmt.Errorf("GOARCH=arm requires explicit GOARM support") + } target := goos + "/" + goarch if _, ok := supported[target]; !ok { continue @@ -858,15 +914,7 @@ func explicitGoReleaserTargets( ) (map[string]struct{}, error) { targets := make(map[string]struct{}) for _, configuredTarget := range configured { - if configuredTarget == "go_first_class" { - for key, target := range supported { - if target.FirstClass { - targets[key] = struct{}{} - } - } - continue - } - if configuredTarget == "go_118_first_class" { + if configuredTarget == "go_first_class" || configuredTarget == "go_118_first_class" { return nil, fmt.Errorf("unsupported target selector %q", configuredTarget) } if strings.Contains(configuredTarget, "{{") { @@ -877,6 +925,9 @@ func explicitGoReleaserTargets( if len(parts) != 2 { return nil, fmt.Errorf("malformed target %q", configuredTarget) } + if parts[1] == "arm" { + return nil, fmt.Errorf("target %q requires explicit GOARM support", configuredTarget) + } target := parts[0] + "/" + parts[1] if _, ok := supported[target]; !ok { return nil, fmt.Errorf("unsupported Go target %q", configuredTarget) @@ -886,6 +937,38 @@ func explicitGoReleaserTargets( return targets, nil } +func buildFlagsSetTags(flags []string) bool { + for _, flag := range flags { + if flag == "-tags" || strings.HasPrefix(flag, "-tags=") { + return true + } + } + return false +} + +func validateGoReleaserBuildEnv(env []string) error { + for _, entry := range env { + name, value, ok := strings.Cut(entry, "=") + if !ok { + continue + } + switch name { + case "CGO_ENABLED": + if value != "0" { + return fmt.Errorf("CGO_ENABLED=%s is unsupported", value) + } + case "GOFLAGS": + if strings.Contains(value, "-tags") { + return fmt.Errorf("GOFLAGS build tags are unsupported") + } + case "GOEXPERIMENT", "GOOS", "GOARCH", "GOARM", "GOAMD64", "GOARM64", + "GOMIPS", "GOMIPS64", "GO386", "GOPPC64", "GORISCV64": + return fmt.Errorf("build environment variable %q is unsupported", name) + } + } + return nil +} + func buildHasMicroarchitectureMatrix(build goReleaserBuild) bool { return len(build.GOARM) > 0 || len(build.GOAMD64) > 0 || @@ -963,7 +1046,7 @@ func TestDecodeAndMergeListedPackages(t *testing.T) { func TestGoListPackagesSeparatesStderr(t *testing.T) { target := goListTarget{GOOS: "linux", GOARCH: "amd64"} - packages, stderr, err := loadPackagesForTarget("", target, func(_ string, _ ...string) *exec.Cmd { + packages, stderr, err := loadPackagesForTarget("", target, "authsidecar", func(_ string, _ ...string) *exec.Cmd { cmd := exec.Command(os.Args[0], "-test.run=^TestGoListCommandHelperProcess$") cmd.Env = append(os.Environ(), "GO_LIST_COMMAND_HELPER=1") return cmd @@ -992,12 +1075,14 @@ func goListPackageGraph(t *testing.T, root string) []listedPackage { t.Helper() packagesByPath := make(map[string]listedPackage) for _, target := range layeringBuildTargets { - for _, pkg := range goListPackages(t, root, target) { - merged := packagesByPath[pkg.ImportPath] - merged.ImportPath = pkg.ImportPath - merged.Imports = mergeStrings(merged.Imports, pkg.Imports) - merged.Deps = mergeStrings(merged.Deps, pkg.Deps) - packagesByPath[pkg.ImportPath] = merged + for _, tags := range layeringBuildTags { + for _, pkg := range goListPackages(t, root, target, tags) { + merged := packagesByPath[pkg.ImportPath] + merged.ImportPath = pkg.ImportPath + merged.Imports = mergeStrings(merged.Imports, pkg.Imports) + merged.Deps = mergeStrings(merged.Deps, pkg.Deps) + packagesByPath[pkg.ImportPath] = merged + } } } @@ -1011,14 +1096,15 @@ func goListPackageGraph(t *testing.T, root string) []listedPackage { return packages } -func goListPackages(t *testing.T, root string, target goListTarget) []listedPackage { +func goListPackages(t *testing.T, root string, target goListTarget, tags string) []listedPackage { t.Helper() - packages, stderr, err := loadPackagesForTarget(root, target, exec.Command) + packages, stderr, err := loadPackagesForTarget(root, target, tags, exec.Command) if err != nil { t.Fatalf( - "GOOS=%s GOARCH=%s go list -json -tags authsidecar ./... failed: %v\n%s", + "GOOS=%s GOARCH=%s tags=%q go list -json ./... failed: %v\n%s", target.GOOS, target.GOARCH, + tags, err, stderr, ) @@ -1029,9 +1115,14 @@ func goListPackages(t *testing.T, root string, target goListTarget) []listedPack func loadPackagesForTarget( root string, target goListTarget, + tags string, newCommand commandFactory, ) ([]listedPackage, string, error) { - args := []string{"list", "-json", "-tags", "authsidecar", "./..."} + args := []string{"list", "-json"} + if tags != "" { + args = append(args, "-tags", tags) + } + args = append(args, "./...") cmd := newCommand("go", args...) cmd.Dir = root env := cmd.Env From abe0d09d4b8e589fc91160729890fb3a0b3aab1a Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 17:05:33 +0800 Subject: [PATCH 09/50] fix(qualitygate): harden layering edge parsing and release-target checks Layering edge parsing and graph coverage: - Reject whitespace-padded exception fields instead of silently trimming them, so a padded row is a malformed row rather than a coerced identity; add a padded-field parse test. - Fail loud when any release target/tag combination lists zero packages, which would otherwise let the layering graph silently under-cover. - Document the build-tag scope (demo tags excluded), the SkipFrom substring semantics, and the toolchain-derived support set behind the drift check. GoReleaser drift checks: - Reject custom build commands and per-target overrides as unsupported. - Detect --tags in addition to -tags when rejecting release build tags. - Reject any GO* build environment variable (except CGO_ENABLED=0) through a single default branch instead of an explicit allowlist. - Validate the GoReleaser global env block, and make the go-list stderr test table-driven across the default and authsidecar graphs. --- internal/qualitygate/deptest/layering_test.go | 137 +++++++++++++++--- 1 file changed, 113 insertions(+), 24 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 98e2a5e6d4..4812b42742 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -42,8 +42,12 @@ type Rule struct { Mode Mode FromPrefix string Denied []string + // ExceptFrom exempts import paths matched exactly. ExceptFrom []string - SkipFrom []string + // SkipFrom exempts any package whose import path contains one of these + // substrings (e.g. "/examples/" to skip demo code anywhere in the tree), + // deliberately looser than the prefix matching used for the other fields. + SkipFrom []string } var rules = []Rule{ @@ -121,6 +125,7 @@ type goListTarget struct { type commandFactory func(name string, args ...string) *exec.Cmd type goReleaserConfig struct { + Env []string `yaml:"env"` Builds []goReleaserBuild `yaml:"builds"` } @@ -141,6 +146,8 @@ type goReleaserBuild struct { Tags []string `yaml:"tags"` Flags []string `yaml:"flags"` Env []string `yaml:"env"` + Command string `yaml:"command"` + Overrides yaml.Node `yaml:"overrides"` Targets []string `yaml:"targets"` Ignore []map[string]any `yaml:"ignore"` Skip bool `yaml:"skip"` @@ -161,6 +168,10 @@ var layeringBuildTargets = []goListTarget{ {GOOS: "windows", GOARCH: "arm64"}, } +// layeringBuildTags is the set of build tags whose import graphs are unioned +// before evaluating the rules. Demo-only tags (authsidecar_demo, +// authsidecar_multi_tenant_demo) are intentionally excluded: that code lives +// under sidecar/server-demo*/ and never sits in a layer any rule governs. var layeringBuildTags = []string{"", "authsidecar"} type layeringEdge struct { @@ -298,6 +309,10 @@ func TestParseLayeringEdges(t *testing.T) { name: "invalid-date", input: "from\tdenied\towner\treason\t2026-02-30\n", }, + { + name: "whitespace-padded-field", + input: "from\t denied \towner\treason\t2026-07-24\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -634,7 +649,14 @@ func TestLayeringBuildTargetsMatchGoReleaser(t *testing.T) { if len(config.Builds) == 0 { t.Fatal(".goreleaser.yml has no builds") } + if err := validateGoReleaserBuildEnv(config.Env); err != nil { + t.Fatalf(".goreleaser.yml global environment: %v", err) + } + // The supported set is derived from the toolchain running this test. + // TestReleaseGoVersionMatchesModule pins the release Go to the go.mod floor, + // so a runner older than the release toolchain (which could omit a + // release-buildable target and under-compute want) is caught there first. output, err := exec.Command("go", "tool", "dist", "list", "-json").Output() if err != nil { t.Fatalf("go tool dist list: %v", err) @@ -812,7 +834,7 @@ func TestGoReleaserBuildTargets(t *testing.T) { { name: "build flag tags fail closed", build: goReleaserBuild{ - Flags: []string{"-tags=feature"}, + Flags: []string{"--tags=feature"}, }, wantErr: true, }, @@ -823,6 +845,20 @@ func TestGoReleaserBuildTargets(t *testing.T) { }, wantErr: true, }, + { + name: "custom build command fails closed", + build: goReleaserBuild{ + Command: "test", + }, + wantErr: true, + }, + { + name: "target overrides fail closed", + build: goReleaserBuild{ + Overrides: yaml.Node{Kind: yaml.MappingNode}, + }, + wantErr: true, + }, { name: "enabled cgo fails closed", build: goReleaserBuild{ @@ -864,6 +900,12 @@ func goReleaserBuildTargets( if build.Tool != "" || build.GoBinary != "" { return nil, fmt.Errorf("custom Go tools are unsupported") } + if build.Command != "" && build.Command != "build" { + return nil, fmt.Errorf("unsupported Go command %q", build.Command) + } + if build.Overrides.Kind != 0 { + return nil, fmt.Errorf("target overrides are unsupported") + } if len(build.Tags) > 0 || buildFlagsSetTags(build.Flags) { return nil, fmt.Errorf("release build tags are unsupported") } @@ -939,7 +981,8 @@ func explicitGoReleaserTargets( func buildFlagsSetTags(flags []string) bool { for _, flag := range flags { - if flag == "-tags" || strings.HasPrefix(flag, "-tags=") { + if flag == "-tags" || strings.HasPrefix(flag, "-tags=") || + flag == "--tags" || strings.HasPrefix(flag, "--tags=") { return true } } @@ -957,12 +1000,10 @@ func validateGoReleaserBuildEnv(env []string) error { if value != "0" { return fmt.Errorf("CGO_ENABLED=%s is unsupported", value) } - case "GOFLAGS": - if strings.Contains(value, "-tags") { - return fmt.Errorf("GOFLAGS build tags are unsupported") + default: + if !strings.HasPrefix(name, "GO") { + continue } - case "GOEXPERIMENT", "GOOS", "GOARCH", "GOARM", "GOAMD64", "GOARM64", - "GOMIPS", "GOMIPS64", "GO386", "GOPPC64", "GORISCV64": return fmt.Errorf("build environment variable %q is unsupported", name) } } @@ -1046,19 +1087,45 @@ func TestDecodeAndMergeListedPackages(t *testing.T) { func TestGoListPackagesSeparatesStderr(t *testing.T) { target := goListTarget{GOOS: "linux", GOARCH: "amd64"} - packages, stderr, err := loadPackagesForTarget("", target, "authsidecar", func(_ string, _ ...string) *exec.Cmd { - cmd := exec.Command(os.Args[0], "-test.run=^TestGoListCommandHelperProcess$") - cmd.Env = append(os.Environ(), "GO_LIST_COMMAND_HELPER=1") - return cmd - }) - if err != nil { - t.Fatalf("loadPackagesForTarget returned an error: %v", err) - } - if !strings.Contains(stderr, "go: downloading example.com/module\n") { - t.Fatalf("loadPackagesForTarget stderr = %q, want module download diagnostic", stderr) + tests := []struct { + name string + tags string + wantArgs []string + }{ + { + name: "default graph", + wantArgs: []string{"list", "-json", "./..."}, + }, + { + name: "authsidecar graph", + tags: "authsidecar", + wantArgs: []string{"list", "-json", "-tags", "authsidecar", "./..."}, + }, } - if len(packages) != 1 || packages[0].ImportPath != "example.com/package" { - t.Fatalf("loadPackagesForTarget returned unexpected packages: %+v", packages) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotName string + var gotArgs []string + packages, stderr, err := loadPackagesForTarget("", target, tt.tags, func(name string, args ...string) *exec.Cmd { + gotName = name + gotArgs = slices.Clone(args) + cmd := exec.Command(os.Args[0], "-test.run=^TestGoListCommandHelperProcess$") + cmd.Env = append(os.Environ(), "GO_LIST_COMMAND_HELPER=1") + return cmd + }) + if err != nil { + t.Fatalf("loadPackagesForTarget returned an error: %v", err) + } + if gotName != "go" || !slices.Equal(gotArgs, tt.wantArgs) { + t.Fatalf("command = %q %q, want go %q", gotName, gotArgs, tt.wantArgs) + } + if !strings.Contains(stderr, "go: downloading example.com/module\n") { + t.Fatalf("loadPackagesForTarget stderr = %q, want module download diagnostic", stderr) + } + if len(packages) != 1 || packages[0].ImportPath != "example.com/package" { + t.Fatalf("loadPackagesForTarget returned unexpected packages: %+v", packages) + } + }) } } @@ -1076,7 +1143,16 @@ func goListPackageGraph(t *testing.T, root string) []listedPackage { packagesByPath := make(map[string]listedPackage) for _, target := range layeringBuildTargets { for _, tags := range layeringBuildTags { - for _, pkg := range goListPackages(t, root, target, tags) { + listed := goListPackages(t, root, target, tags) + if len(listed) == 0 { + t.Fatalf( + "GOOS=%s GOARCH=%s tags=%q go list -json ./... returned no packages; the layering graph would silently under-cover", + target.GOOS, + target.GOARCH, + tags, + ) + } + for _, pkg := range listed { merged := packagesByPath[pkg.ImportPath] merged.ImportPath = pkg.ImportPath merged.Imports = mergeStrings(merged.Imports, pkg.Imports) @@ -1318,11 +1394,15 @@ func parseLayeringEdges(r io.Reader) ([]seededLayeringEdge, error) { problems = append(problems, malformedLayeringEdge(line)) continue } - for i := range parts { - parts[i] = strings.TrimSpace(parts[i]) + // Reject rather than trim: an import path never carries surrounding + // whitespace, so a padded field is a malformed row, not one to + // silently normalize into a different identity. + if hasBlank(parts...) || hasSurroundingWhitespace(parts...) { + problems = append(problems, malformedLayeringEdge(line)) + continue } addedAt, dateErr := time.Parse(time.DateOnly, parts[4]) - if hasBlank(parts...) || dateErr != nil { + if dateErr != nil { problems = append(problems, malformedLayeringEdge(line)) continue } @@ -1360,6 +1440,15 @@ func hasBlank(values ...string) bool { return false } +func hasSurroundingWhitespace(values ...string) bool { + for _, value := range values { + if value != strings.TrimSpace(value) { + return true + } + } + return false +} + func malformedLayeringEdge(line int) string { return fmt.Sprintf( "line %d: layering edge row must have five tab-separated non-empty fields with added_at in YYYY-MM-DD format", From d48c218d0dc33cec0a463c1e53d6e0b80b7ee048 Mon Sep 17 00:00:00 2001 From: shanglei Date: Fri, 24 Jul 2026 17:49:33 +0800 Subject: [PATCH 10/50] fix: close layering quality gate gaps --- .../qualitygate/deptest/layering-edges.txt | 2 + internal/qualitygate/deptest/layering_test.go | 154 ++++++++++++++++-- scripts/check-layering-ratchet.sh | 19 ++- scripts/check-layering-ratchet.test.sh | 41 ++++- 4 files changed, 189 insertions(+), 27 deletions(-) diff --git a/internal/qualitygate/deptest/layering-edges.txt b/internal/qualitygate/deptest/layering-edges.txt index 10c8e36508..5301b6aaa2 100644 --- a/internal/qualitygate/deptest/layering-edges.txt +++ b/internal/qualitygate/deptest/layering-edges.txt @@ -16,6 +16,8 @@ github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/i github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/vfs arch-migration extension pre-module-split (via core/envvars) 2026-07-24 github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/vfs/localfileio arch-migration extension pre-module-split (via core/envvars) 2026-07-24 github.com/larksuite/cli/extension/transport/sidecar github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 +github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events root aggregates the existing im conversion dependency 2026-07-24 +github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/common arch-migration events root aggregates the existing im conversion dependency 2026-07-24 github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events->shortcuts inversion via convert_lib 2026-07-24 github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/common arch-migration events->shortcuts inversion via convert_lib 2026-07-24 github.com/larksuite/cli/shortcuts/im github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index 4812b42742..fa7ffda226 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -54,20 +54,20 @@ var rules = []Rule{ { Name: "extension-zero-internal", Mode: Transitive, - FromPrefix: modulePath + "/extension/", - Denied: []string{modulePath + "/internal/"}, + FromPrefix: modulePath + "/extension", + Denied: []string{modulePath + "/internal"}, SkipFrom: []string{"/examples/"}, }, { Name: "events-no-shortcuts", Mode: Transitive, - FromPrefix: modulePath + "/events/", - Denied: []string{modulePath + "/shortcuts/"}, + FromPrefix: modulePath + "/events", + Denied: []string{modulePath + "/shortcuts"}, }, { Name: "shortcuts-runtime-gate", Mode: Direct, - FromPrefix: modulePath + "/shortcuts/", + FromPrefix: modulePath + "/shortcuts", Denied: []string{ modulePath + "/internal/auth", modulePath + "/internal/keychain", @@ -99,7 +99,7 @@ var rules = []Rule{ { Name: "internal-no-upper", Mode: Direct, - FromPrefix: modulePath + "/internal/", + FromPrefix: modulePath + "/internal", Denied: []string{ modulePath + "/cmd", modulePath + "/shortcuts", @@ -413,20 +413,20 @@ func TestLayeringRuleContracts(t *testing.T) { { Name: "extension-zero-internal", Mode: Transitive, - FromPrefix: modulePath + "/extension/", - Denied: []string{modulePath + "/internal/"}, + FromPrefix: modulePath + "/extension", + Denied: []string{modulePath + "/internal"}, SkipFrom: []string{"/examples/"}, }, { Name: "events-no-shortcuts", Mode: Transitive, - FromPrefix: modulePath + "/events/", - Denied: []string{modulePath + "/shortcuts/"}, + FromPrefix: modulePath + "/events", + Denied: []string{modulePath + "/shortcuts"}, }, { Name: "shortcuts-runtime-gate", Mode: Direct, - FromPrefix: modulePath + "/shortcuts/", + FromPrefix: modulePath + "/shortcuts", Denied: []string{ modulePath + "/internal/auth", modulePath + "/internal/keychain", @@ -458,7 +458,7 @@ func TestLayeringRuleContracts(t *testing.T) { { Name: "internal-no-upper", Mode: Direct, - FromPrefix: modulePath + "/internal/", + FromPrefix: modulePath + "/internal", Denied: []string{ modulePath + "/cmd", modulePath + "/shortcuts", @@ -613,6 +613,78 @@ func TestLayeringRuleContracts(t *testing.T) { } } +func TestLayeringRulesCoverRootPackages(t *testing.T) { + tests := []struct { + name string + ruleName string + pkg listedPackage + wantDenied string + }{ + { + name: "extension-root", + ruleName: "extension-zero-internal", + pkg: listedPackage{ + ImportPath: modulePath + "/extension", + Deps: []string{modulePath + "/internal"}, + }, + wantDenied: modulePath + "/internal", + }, + { + name: "events-root", + ruleName: "events-no-shortcuts", + pkg: listedPackage{ + ImportPath: modulePath + "/events", + Deps: []string{modulePath + "/shortcuts"}, + }, + wantDenied: modulePath + "/shortcuts", + }, + { + name: "shortcuts-root", + ruleName: "shortcuts-runtime-gate", + pkg: listedPackage{ + ImportPath: modulePath + "/shortcuts", + Imports: []string{modulePath + "/internal/client"}, + }, + wantDenied: modulePath + "/internal/client", + }, + { + name: "internal-root", + ruleName: "internal-no-upper", + pkg: listedPackage{ + ImportPath: modulePath + "/internal", + Imports: []string{modulePath + "/events"}, + }, + wantDenied: modulePath + "/events", + }, + } + + rulesByName := make(map[string]Rule, len(rules)) + for _, rule := range rules { + rulesByName[rule.Name] = rule + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rule, ok := rulesByName[tt.ruleName] + if !ok { + t.Fatalf("missing rule %q", tt.ruleName) + } + violations := evaluateLayeringRule([]listedPackage{tt.pkg}, rule) + if len(violations) != 1 { + t.Fatalf("evaluateLayeringRule returned %d violations, want 1: %+v", len(violations), violations) + } + if violations[0].From != tt.pkg.ImportPath || violations[0].Denied != tt.wantDenied { + t.Fatalf( + "evaluateLayeringRule returned edge (%q, %q), want (%q, %q)", + violations[0].From, + violations[0].Denied, + tt.pkg.ImportPath, + tt.wantDenied, + ) + } + }) + } +} + func TestLayeringBuildTargets(t *testing.T) { want := []goListTarget{ {GOOS: "linux", GOARCH: "amd64"}, @@ -845,6 +917,37 @@ func TestGoReleaserBuildTargets(t *testing.T) { }, wantErr: true, }, + { + name: "templated build environment fails closed", + build: goReleaserBuild{ + Env: []string{`{{- if eq .Os "linux" }}GOFLAGS=-tags=feature{{- end }}`}, + }, + wantErr: true, + }, + { + name: "templated build flags fail closed", + build: goReleaserBuild{ + Flags: []string{`{{- if eq .Os "linux" }}-tags=feature{{- end }}`}, + }, + wantErr: true, + }, + { + name: "templated target matrix fails closed", + build: goReleaserBuild{ + GOOS: []string{`{{ .Env.GOOS }}`}, + GOARCH: []string{"amd64"}, + }, + wantErr: true, + }, + { + name: "templated ignore fails closed", + build: goReleaserBuild{ + GOOS: []string{"linux"}, + GOARCH: []string{"amd64"}, + Ignore: []map[string]any{{"goos": `{{ .Env.GOOS }}`}}, + }, + wantErr: true, + }, { name: "custom build command fails closed", build: goReleaserBuild{ @@ -906,6 +1009,12 @@ func goReleaserBuildTargets( if build.Overrides.Kind != 0 { return nil, fmt.Errorf("target overrides are unsupported") } + if containsGoReleaserTemplate(build.GOOS) || containsGoReleaserTemplate(build.GOARCH) { + return nil, fmt.Errorf("templated release target matrices are unsupported") + } + if containsGoReleaserTemplate(build.Flags) { + return nil, fmt.Errorf("templated release build flags are unsupported") + } if len(build.Tags) > 0 || buildFlagsSetTags(build.Flags) { return nil, fmt.Errorf("release build tags are unsupported") } @@ -959,7 +1068,7 @@ func explicitGoReleaserTargets( if configuredTarget == "go_first_class" || configuredTarget == "go_118_first_class" { return nil, fmt.Errorf("unsupported target selector %q", configuredTarget) } - if strings.Contains(configuredTarget, "{{") { + if hasGoReleaserTemplate(configuredTarget) { return nil, fmt.Errorf("templated target %q is unsupported", configuredTarget) } @@ -989,8 +1098,24 @@ func buildFlagsSetTags(flags []string) bool { return false } +func containsGoReleaserTemplate(values []string) bool { + for _, value := range values { + if hasGoReleaserTemplate(value) { + return true + } + } + return false +} + +func hasGoReleaserTemplate(value string) bool { + return strings.Contains(value, "{{") || strings.Contains(value, "}}") +} + func validateGoReleaserBuildEnv(env []string) error { for _, entry := range env { + if hasGoReleaserTemplate(entry) { + return fmt.Errorf("templated build environment entries are unsupported") + } name, value, ok := strings.Cut(entry, "=") if !ok { continue @@ -1053,6 +1178,9 @@ func optionalString(values map[string]any, key string) (string, error) { if !ok { return "", fmt.Errorf("ignore selector %q must be a string", key) } + if hasGoReleaserTemplate(text) { + return "", fmt.Errorf("templated ignore selector %q is unsupported", key) + } return text, nil } diff --git a/scripts/check-layering-ratchet.sh b/scripts/check-layering-ratchet.sh index 9cbc82a0f7..ad76b2cabd 100644 --- a/scripts/check-layering-ratchet.sh +++ b/scripts/check-layering-ratchet.sh @@ -20,11 +20,16 @@ layering_ratchet_extract_keys() { printf "Malformed layering ratchet row at %s:%d: expected five tab-separated fields.\n", FILENAME, FNR > "/dev/stderr" exit 2 } - from = trim($1) - denied = trim($2) - owner = trim($3) - reason = trim($4) - added_at = trim($5) + from = $1 + denied = $2 + owner = $3 + reason = $4 + added_at = $5 + sub(/\r+$/, "", added_at) + if (from != trim(from) || denied != trim(denied) || owner != trim(owner) || reason != trim(reason) || added_at != trim(added_at)) { + printf "Malformed layering ratchet row at %s:%d: fields must not have surrounding whitespace.\n", FILENAME, FNR > "/dev/stderr" + exit 2 + } if (from == "" || denied == "" || owner == "" || reason == "" || added_at !~ /^[0-9]{4}-[0-9]{2}-[0-9]{2}$/) { printf "Malformed layering ratchet row at %s:%d: fields must be non-empty and added_at must use YYYY-MM-DD.\n", FILENAME, FNR > "/dev/stderr" exit 2 @@ -82,8 +87,8 @@ layering_ratchet_main() ( fi local base_revision="${1:-${QUALITY_GATE_CHANGED_FROM:-}}" - local approved_initial_count="${2:-37}" - local approved_initial_hash="${3:-cf61e4149cb4d539a1430f4a4266b91f972c253cedd300830c94e35dda1f0265}" + local approved_initial_count="${2:-39}" + local approved_initial_hash="${3:-5636d50d10b9de1e08dc9f06cd66671b3f438650fa5b7f28b95aa7d5a69a1c21}" if [[ -z "$base_revision" ]]; then echo "Layering ratchet requires a base revision." >&2 return 1 diff --git a/scripts/check-layering-ratchet.test.sh b/scripts/check-layering-ratchet.test.sh index 39744b96b3..08af81b3ac 100644 --- a/scripts/check-layering-ratchet.test.sh +++ b/scripts/check-layering-ratchet.test.sh @@ -190,6 +190,31 @@ test_malformed_and_missing_current_file_fail() { expect_fail "$dir" "$base" "Layering ratchet file is missing" } +test_surrounding_whitespace_fails() { + local dir="$tmp/surrounding-whitespace" + git_init "$dir" + write_rows "$dir" from/a denied/a + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + printf '# from\tdenied\towner\treason\tadded_at\nfrom/a\t denied/a \towner\treason\t2026-07-24\n' >"$dir/$ratchet_file" + expect_fail "$dir" "$base" "fields must not have surrounding whitespace" + expect_sourced_main_fail "$dir" "$base" "fields must not have surrounding whitespace" +} + +test_crlf_rows_pass() { + local dir="$tmp/crlf" + git_init "$dir" + write_rows "$dir" from/a denied/a + commit_ratchet "$dir" + local base + base="$(git -C "$dir" rev-parse HEAD)" + + printf '# from\tdenied\towner\treason\tadded_at\r\nfrom/a\tdenied/a\towner\treason\t2026-07-24\r\n' >"$dir/$ratchet_file" + expect_pass "$dir" "$base" +} + test_duplicate_key_fails() { local dir="$tmp/duplicate" git_init "$dir" @@ -213,7 +238,7 @@ test_bootstrap_requires_the_approved_snapshot() { local args=() local index - for index in $(seq 1 37); do + for index in $(seq 1 39); do args+=("from/$index" "denied/$index") done write_rows "$dir" "${args[@]}" @@ -221,16 +246,16 @@ test_bootstrap_requires_the_approved_snapshot() { bootstrap_keys "$dir/$ratchet_file" >"$keys_file" local expected_hash expected_hash="$(hash_file "$keys_file")" - expect_bootstrap_pass "$dir" "$base" 37 "$expected_hash" + expect_bootstrap_pass "$dir" "$base" 39 "$expected_hash" - args[72]="from/replacement" + args[76]="from/replacement" write_rows "$dir" "${args[@]}" - expect_bootstrap_fail "$dir" "$base" 37 "$expected_hash" + expect_bootstrap_fail "$dir" "$base" 39 "$expected_hash" - args[72]="from/37" - args+=("from/38" "denied/38") + args[76]="from/39" + args+=("from/40" "denied/40") write_rows "$dir" "${args[@]}" - expect_bootstrap_fail "$dir" "$base" 37 "$expected_hash" + expect_bootstrap_fail "$dir" "$base" 39 "$expected_hash" } test_invalid_base_fails() { @@ -246,6 +271,8 @@ test_deletion_passes test_addition_fails test_equal_count_replacement_fails test_malformed_and_missing_current_file_fail +test_surrounding_whitespace_fails +test_crlf_rows_pass test_duplicate_key_fails test_bootstrap_requires_the_approved_snapshot test_invalid_base_fails From 820305536c68604364e235221bf2c01c08153afd Mon Sep 17 00:00:00 2001 From: shanglei Date: Sat, 25 Jul 2026 12:18:26 +0800 Subject: [PATCH 11/50] fix(qualitygate): scope the examples exemption to wrapper mains The extension rule skipped any package whose import path contained "/examples/", which let the gate miss two things: a directory named examples anywhere under extension escaped the rule outright, and the sanctioned demos were exempt from every denial rather than only from the internal packages they inherit through cmd. - Drop SkipFrom (and containsAny) so no rule can exempt by directory name. - Exempt the two wrapper-main demos from extension-zero-internal by exact import path. Their cmd import is the pattern they exist to demonstrate, and seeding those edges instead would wedge the ratchet: the edges track cmd's transitive set, so a new internal package under cmd would demand a new row that check-layering-ratchet.sh refuses by design. - Add examples-surface-only: demos may consume cmd and extension/platform but must not directly import internal or shortcuts. Zero violations today. layering-edges.txt stays at 39 rows, so the ratchet bootstrap snapshot still matches. --- internal/qualitygate/deptest/layering_test.go | 99 +++++++++++++++---- 1 file changed, 79 insertions(+), 20 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index fa7ffda226..be9e5c957d 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -44,19 +44,46 @@ type Rule struct { Denied []string // ExceptFrom exempts import paths matched exactly. ExceptFrom []string - // SkipFrom exempts any package whose import path contains one of these - // substrings (e.g. "/examples/" to skip demo code anywhere in the tree), - // deliberately looser than the prefix matching used for the other fields. - SkipFrom []string } +// examplesPrefix is the plugin-SDK example tree. Each subdirectory is a +// standalone main package demonstrating the sanctioned wrapper-main pattern: +// register a plugin from init, then hand the process to cmd.Execute. The +// customer forks built by tests/plugin_e2e/harness.go have the same shape. +const examplesPrefix = modulePath + "/extension/platform/examples" + var rules = []Rule{ { Name: "extension-zero-internal", Mode: Transitive, FromPrefix: modulePath + "/extension", Denied: []string{modulePath + "/internal"}, - SkipFrom: []string{"/examples/"}, + // The wrapper-main demos import cmd deliberately — that fork is the + // thing they exist to show — so every internal package they reach is + // inherited through cmd rather than chosen by the demo. Exempting + // them by exact path (never by directory name) keeps two properties: + // a future examples/ subdirectory inherits no exemption, and + // examples-surface-only still stops a demo from reaching down on its + // own. Listing these edges in layering-edges.txt instead would also + // wedge the ratchet: they track cmd's transitive set, so any new + // internal package under cmd would demand a new row, which + // check-layering-ratchet.sh refuses by design. + ExceptFrom: []string{ + examplesPrefix + "/audit-observer", + examplesPrefix + "/readonly-policy", + }, + }, + { + // Demos may consume the assembled CLI (cmd) and the public SDK + // (extension/platform); reaching past those into internals or + // business commands would teach plugin authors the wrong entry point. + Name: "examples-surface-only", + Mode: Direct, + FromPrefix: examplesPrefix, + Denied: []string{ + modulePath + "/internal", + modulePath + "/shortcuts", + }, }, { Name: "events-no-shortcuts", @@ -415,7 +442,19 @@ func TestLayeringRuleContracts(t *testing.T) { Mode: Transitive, FromPrefix: modulePath + "/extension", Denied: []string{modulePath + "/internal"}, - SkipFrom: []string{"/examples/"}, + ExceptFrom: []string{ + examplesPrefix + "/audit-observer", + examplesPrefix + "/readonly-policy", + }, + }, + { + Name: "examples-surface-only", + Mode: Direct, + FromPrefix: examplesPrefix, + Denied: []string{ + modulePath + "/internal", + modulePath + "/shortcuts", + }, }, { Name: "events-no-shortcuts", @@ -481,7 +520,7 @@ func TestLayeringRuleContracts(t *testing.T) { wantDenied string }{ { - name: "extension-transitive-denial-and-example-scope-out", + name: "extension-transitive-denial-and-wrapper-main-exemption", ruleName: "extension-zero-internal", packages: []listedPackage{ { @@ -489,13 +528,45 @@ func TestLayeringRuleContracts(t *testing.T) { Deps: []string{modulePath + "/internal/core"}, }, { - ImportPath: modulePath + "/extension/platform/examples/demo", + ImportPath: examplesPrefix + "/audit-observer", Deps: []string{modulePath + "/internal/core"}, }, }, wantFrom: modulePath + "/extension/sdk", wantDenied: modulePath + "/internal/core", }, + { + // The exemption is per exact path, so parking code in a new + // directory under examples/ buys no cover. + name: "extension-still-covers-unlisted-example-directories", + ruleName: "extension-zero-internal", + packages: []listedPackage{ + { + ImportPath: examplesPrefix + "/demo", + Deps: []string{modulePath + "/internal/core"}, + }, + }, + wantFrom: examplesPrefix + "/demo", + wantDenied: modulePath + "/internal/core", + }, + { + // cmd is the demo's whole point and stays legal; reaching past it + // into internals does not. + name: "examples-may-wrap-cmd-but-not-reach-internals", + ruleName: "examples-surface-only", + packages: []listedPackage{ + { + ImportPath: examplesPrefix + "/audit-observer", + Imports: []string{ + modulePath + "/cmd", + modulePath + "/extension/platform", + modulePath + "/internal/keychain", + }, + }, + }, + wantFrom: examplesPrefix + "/audit-observer", + wantDenied: modulePath + "/internal/keychain", + }, { name: "events-transitive-denial", ruleName: "events-no-shortcuts", @@ -1405,9 +1476,6 @@ func evaluateLayeringRule(packages []listedPackage, rule Rule) []layeringViolati if slices.Contains(rule.ExceptFrom, pkg.ImportPath) { continue } - if containsAny(pkg.ImportPath, rule.SkipFrom) { - continue - } dependencies := pkg.Imports if rule.Mode == Transitive { @@ -1454,15 +1522,6 @@ func matchesAnyPackagePrefix(prefixes []string, importPath string) bool { return false } -func containsAny(value string, substrings []string) bool { - for _, substring := range substrings { - if strings.Contains(value, substring) { - return true - } - } - return false -} - func findUnseededLayeringViolations( actual []layeringViolation, seeded map[layeringEdge]seededLayeringEdge, From 217f4e55670f78d57c781aab9d456a5e27f32dd9 Mon Sep 17 00:00:00 2001 From: shanglei Date: Sat, 25 Jul 2026 14:30:29 +0800 Subject: [PATCH 12/50] fix(qualitygate): pin the examples surface with an allowlist examples-surface-only promised that demos may consume only the assembled CLI and the public plugin SDK, but it enforced two denied prefixes instead, so every tree nobody thought to deny was permitted. A demo importing `events`, `errs` or a `cmd` subpackage passed the gate, and because extension-zero-internal exempts these packages from the transitive check, nothing examined what those imports dragged in either. The exemption was therefore unbounded in what it covered, the same defect as the directory-name skip it replaced. - Add Rule.AllowedRepoDeps, which inverts the check: any dependency inside this module that is not listed is a violation. Standard library and third-party packages, including same-organisation modules that are not this one, stay outside the rule. - Pin examples-surface-only to exactly `cmd` and `extension/platform`, so the rule name matches what it enforces and the inherited chain stays bounded by a direct surface of two packages. - Cover the reproducers as contract cases: other repository trees, `cmd` subpackages, other `extension` subtrees, and the module root are rejected, while the two allowed packages plus non-module imports are not. layering-edges.txt stays at 39 rows; the demos already import only the two allowed packages. --- internal/qualitygate/deptest/layering_test.go | 107 ++++++++++++++++-- 1 file changed, 95 insertions(+), 12 deletions(-) diff --git a/internal/qualitygate/deptest/layering_test.go b/internal/qualitygate/deptest/layering_test.go index be9e5c957d..2c27ef8d01 100644 --- a/internal/qualitygate/deptest/layering_test.go +++ b/internal/qualitygate/deptest/layering_test.go @@ -44,6 +44,13 @@ type Rule struct { Denied []string // ExceptFrom exempts import paths matched exactly. ExceptFrom []string + // AllowedRepoDeps inverts the check. When set, every dependency inside this + // module that is not listed here (matched exactly) is a violation and Denied + // is unused. Dependencies outside the module - standard library and + // third-party packages - are never considered. Prefer this over Denied + // whenever the rule name promises a surface rather than a blocklist, so the + // guarantee cannot drift as the repository grows new top-level trees. + AllowedRepoDeps []string } // examplesPrefix is the plugin-SDK example tree. Each subdirectory is a @@ -74,15 +81,19 @@ var rules = []Rule{ }, }, { - // Demos may consume the assembled CLI (cmd) and the public SDK - // (extension/platform); reaching past those into internals or - // business commands would teach plugin authors the wrong entry point. + // Demos exist to show the wrapper-main pattern, so the assembled CLI and + // the public plugin SDK are the only repository packages they may reach + // for. This is an allowlist rather than a few denied prefixes because + // extension-zero-internal exempts these packages from the transitive + // check: pinning the direct surface is the only thing that keeps the + // inherited chain bounded, and a blocklist would silently permit every + // tree nobody thought to deny (events, errs, cmd subpackages). Name: "examples-surface-only", Mode: Direct, FromPrefix: examplesPrefix, - Denied: []string{ - modulePath + "/internal", - modulePath + "/shortcuts", + AllowedRepoDeps: []string{ + modulePath + "/cmd", + modulePath + "/extension/platform", }, }, { @@ -451,9 +462,9 @@ func TestLayeringRuleContracts(t *testing.T) { Name: "examples-surface-only", Mode: Direct, FromPrefix: examplesPrefix, - Denied: []string{ - modulePath + "/internal", - modulePath + "/shortcuts", + AllowedRepoDeps: []string{ + modulePath + "/cmd", + modulePath + "/extension/platform", }, }, { @@ -550,14 +561,17 @@ func TestLayeringRuleContracts(t *testing.T) { wantDenied: modulePath + "/internal/core", }, { - // cmd is the demo's whole point and stays legal; reaching past it - // into internals does not. + // cmd and the plugin SDK are the demo's whole point and stay legal. + // Standard library and third-party imports are outside the rule, + // including a same-organisation module that is not this one. name: "examples-may-wrap-cmd-but-not-reach-internals", ruleName: "examples-surface-only", packages: []listedPackage{ { ImportPath: examplesPrefix + "/audit-observer", Imports: []string{ + "context", + "github.com/larksuite/oapi-sdk-go/v3", modulePath + "/cmd", modulePath + "/extension/platform", modulePath + "/internal/keychain", @@ -567,6 +581,57 @@ func TestLayeringRuleContracts(t *testing.T) { wantFrom: examplesPrefix + "/audit-observer", wantDenied: modulePath + "/internal/keychain", }, + { + // The allowlist covers trees nobody thought to deny; a blocklist of + // internal and shortcuts would have let this through. + name: "examples-reject-other-repository-trees", + ruleName: "examples-surface-only", + packages: []listedPackage{ + { + ImportPath: examplesPrefix + "/audit-observer", + Imports: []string{modulePath + "/cmd", modulePath + "/events"}, + }, + }, + wantFrom: examplesPrefix + "/audit-observer", + wantDenied: modulePath + "/events", + }, + { + // Only the assembly root is public surface, not its subpackages. + name: "examples-reject-cmd-subpackages", + ruleName: "examples-surface-only", + packages: []listedPackage{ + { + ImportPath: examplesPrefix + "/audit-observer", + Imports: []string{modulePath + "/cmd", modulePath + "/cmd/api"}, + }, + }, + wantFrom: examplesPrefix + "/audit-observer", + wantDenied: modulePath + "/cmd/api", + }, + { + name: "examples-reject-other-extension-subtrees", + ruleName: "examples-surface-only", + packages: []listedPackage{ + { + ImportPath: examplesPrefix + "/audit-observer", + Imports: []string{modulePath + "/extension/fileio"}, + }, + }, + wantFrom: examplesPrefix + "/audit-observer", + wantDenied: modulePath + "/extension/fileio", + }, + { + name: "examples-reject-the-module-root", + ruleName: "examples-surface-only", + packages: []listedPackage{ + { + ImportPath: examplesPrefix + "/audit-observer", + Imports: []string{modulePath}, + }, + }, + wantFrom: examplesPrefix + "/audit-observer", + wantDenied: modulePath, + }, { name: "events-transitive-denial", ruleName: "events-no-shortcuts", @@ -1482,7 +1547,7 @@ func evaluateLayeringRule(packages []listedPackage, rule Rule) []layeringViolati dependencies = pkg.Deps } for _, dependency := range dependencies { - if !matchesAnyPackagePrefix(rule.Denied, dependency) { + if !ruleRejectsDependency(rule, dependency) { continue } violations = append(violations, layeringViolation{ @@ -1503,6 +1568,24 @@ func evaluateLayeringRule(packages []listedPackage, rule Rule) []layeringViolati return violations } +// ruleRejectsDependency reports whether dependency breaks rule, reading the +// allowlist when the rule declares one and the denied prefixes otherwise. +func ruleRejectsDependency(rule Rule, dependency string) bool { + if len(rule.AllowedRepoDeps) == 0 { + return matchesAnyPackagePrefix(rule.Denied, dependency) + } + if !isRepoPackage(dependency) { + return false + } + return !slices.Contains(rule.AllowedRepoDeps, dependency) +} + +// isRepoPackage reports whether importPath belongs to this module rather than +// the standard library or a third-party dependency. +func isRepoPackage(importPath string) bool { + return importPath == modulePath || matchesPackagePrefix(modulePath+"/", importPath) +} + func matchesPackagePrefix(prefix, importPath string) bool { if importPath == prefix { return true From 8ba2431192f1474761ceee0f97373d0c0dd42518 Mon Sep 17 00:00:00 2001 From: shanglei Date: Sat, 25 Jul 2026 16:52:10 +0800 Subject: [PATCH 13/50] refactor(extension): remove internal package dependencies --- cmd/root_integration_test.go | 12 +- cmd/startup_brand.go | 4 +- envnames/envnames.go | 18 +++ extension/credential/env/env.go | 41 ++++--- extension/credential/env/env_test.go | 108 +++++++++--------- extension/credential/sidecar/provider.go | 35 +++--- extension/credential/sidecar/provider_test.go | 44 +++---- extension/transport/sidecar/interceptor.go | 12 +- internal/cmdutil/factory_default_test.go | 48 ++++---- internal/cmdutil/factory_proxy_warn_test.go | 12 +- internal/cmdutil/factory_test.go | 4 +- internal/credential/integration_test.go | 16 +-- internal/envvars/envvars.go | 13 --- internal/envvars/read.go | 3 +- internal/envvars/read_test.go | 12 +- .../qualitygate/deptest/layering-edges.txt | 17 --- main_noauthsidecar.go | 6 +- main_noauthsidecar_test.go | 6 +- sidecar/server-demo/handler_test.go | 14 +-- sidecar/server-demo/main.go | 14 +-- .../server-multi-tenant-demo/handler_test.go | 8 +- sidecar/server-multi-tenant-demo/main.go | 14 +-- 22 files changed, 232 insertions(+), 229 deletions(-) create mode 100644 envnames/envnames.go diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index 11a6b24c84..096eac9a10 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -14,11 +14,11 @@ import ( "github.com/larksuite/cli/cmd/api" "github.com/larksuite/cli/cmd/auth" "github.com/larksuite/cli/cmd/service" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/internal/apicatalog" "github.com/larksuite/cli/internal/build" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/meta" "github.com/larksuite/cli/internal/output" @@ -157,11 +157,11 @@ func strictModeFixtureCatalog() apicatalog.Catalog { func newStrictModeDefaultFactory(t *testing.T, profile string, mode core.StrictMode) (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffer) { t.Helper() - t.Setenv(envvars.CliAppID, "") - t.Setenv(envvars.CliAppSecret, "") - t.Setenv(envvars.CliUserAccessToken, "") - t.Setenv(envvars.CliTenantAccessToken, "") - t.Setenv(envvars.CliDefaultAs, "") + t.Setenv(envnames.CliAppID, "") + t.Setenv(envnames.CliAppSecret, "") + t.Setenv(envnames.CliUserAccessToken, "") + t.Setenv(envnames.CliTenantAccessToken, "") + t.Setenv(envnames.CliDefaultAs, "") dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) diff --git a/cmd/startup_brand.go b/cmd/startup_brand.go index de73ff9b22..80f3761275 100644 --- a/cmd/startup_brand.go +++ b/cmd/startup_brand.go @@ -6,8 +6,8 @@ package cmd import ( "os" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/envvars" ) // ResolveStartupBrand resolves the brand before the command tree is built, so @@ -16,7 +16,7 @@ import ( // environment, then the active profile's raw config entry — without touching // the keychain (no secrets are needed to know the brand). func ResolveStartupBrand(profile string) core.LarkBrand { - if raw := os.Getenv(envvars.CliBrand); raw != "" { + if raw := os.Getenv(envnames.CliBrand); raw != "" { return core.ParseBrand(raw) } if cfg, err := core.LoadMultiAppConfig(); err == nil { diff --git a/envnames/envnames.go b/envnames/envnames.go new file mode 100644 index 0000000000..6734affc31 --- /dev/null +++ b/envnames/envnames.go @@ -0,0 +1,18 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package envnames defines environment variable names shared by the CLI and +// its public extension packages. +package envnames + +const ( + CliAppID = "LARKSUITE_CLI_APP_ID" + CliAppSecret = "LARKSUITE_CLI_APP_SECRET" + CliBrand = "LARKSUITE_CLI_BRAND" + CliUserAccessToken = "LARKSUITE_CLI_USER_ACCESS_TOKEN" + CliTenantAccessToken = "LARKSUITE_CLI_TENANT_ACCESS_TOKEN" + CliDefaultAs = "LARKSUITE_CLI_DEFAULT_AS" + CliStrictMode = "LARKSUITE_CLI_STRICT_MODE" + CliAuthProxy = "LARKSUITE_CLI_AUTH_PROXY" + CliProxyKey = "LARKSUITE_CLI_PROXY_KEY" +) diff --git a/extension/credential/env/env.go b/extension/credential/env/env.go index 5458125689..9af1693c58 100644 --- a/extension/credential/env/env.go +++ b/extension/credential/env/env.go @@ -7,10 +7,10 @@ import ( "context" "fmt" "os" + "strings" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/extension/credential" - "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/envvars" ) // Provider resolves credentials from environment variables. @@ -19,33 +19,33 @@ type Provider struct{} func (p *Provider) Name() string { return "env" } func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, error) { - appID := os.Getenv(envvars.CliAppID) - appSecret := os.Getenv(envvars.CliAppSecret) - hasUAT := os.Getenv(envvars.CliUserAccessToken) != "" - hasTAT := os.Getenv(envvars.CliTenantAccessToken) != "" + appID := os.Getenv(envnames.CliAppID) + appSecret := os.Getenv(envnames.CliAppSecret) + hasUAT := os.Getenv(envnames.CliUserAccessToken) != "" + hasTAT := os.Getenv(envnames.CliTenantAccessToken) != "" if appID == "" && appSecret == "" { switch { case hasUAT: - return nil, &credential.BlockError{Provider: "env", Reason: envvars.CliUserAccessToken + " is set but " + envvars.CliAppID + " is missing"} + return nil, &credential.BlockError{Provider: "env", Reason: envnames.CliUserAccessToken + " is set but " + envnames.CliAppID + " is missing"} case hasTAT: - return nil, &credential.BlockError{Provider: "env", Reason: envvars.CliTenantAccessToken + " is set but " + envvars.CliAppID + " is missing"} + return nil, &credential.BlockError{Provider: "env", Reason: envnames.CliTenantAccessToken + " is set but " + envnames.CliAppID + " is missing"} default: return nil, nil } } if appID == "" { - return nil, &credential.BlockError{Provider: "env", Reason: envvars.CliAppSecret + " is set but " + envvars.CliAppID + " is missing"} + return nil, &credential.BlockError{Provider: "env", Reason: envnames.CliAppSecret + " is set but " + envnames.CliAppID + " is missing"} } if appSecret == "" && !hasUAT && !hasTAT { return nil, &credential.BlockError{ Provider: "env", - Reason: envvars.CliAppID + " is set but no app secret or access token is available", + Reason: envnames.CliAppID + " is set but no app secret or access token is available", } } - brand := credential.Brand(core.ParseBrand(os.Getenv(envvars.CliBrand))) + brand := parseBrand(os.Getenv(envnames.CliBrand)) acct := &credential.Account{AppID: appID, AppSecret: appSecret, Brand: brand} - switch id := credential.Identity(os.Getenv(envvars.CliDefaultAs)); id { + switch id := credential.Identity(os.Getenv(envnames.CliDefaultAs)); id { case "", credential.IdentityAuto: acct.DefaultAs = id case credential.IdentityUser, credential.IdentityBot: @@ -53,12 +53,12 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err default: return nil, &credential.BlockError{ Provider: "env", - Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envvars.CliDefaultAs, id), + Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envnames.CliDefaultAs, id), } } // Explicit strict mode policy takes priority - switch strictMode := os.Getenv(envvars.CliStrictMode); strictMode { + switch strictMode := os.Getenv(envnames.CliStrictMode); strictMode { case "bot": acct.SupportedIdentities = credential.SupportsBot case "user": @@ -76,7 +76,7 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err default: return nil, &credential.BlockError{ Provider: "env", - Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envvars.CliStrictMode, strictMode), + Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envnames.CliStrictMode, strictMode), } } @@ -96,9 +96,9 @@ func (p *Provider) ResolveToken(ctx context.Context, req credential.TokenSpec) ( var envKey string switch req.Type { case credential.TokenTypeUAT: - envKey = envvars.CliUserAccessToken + envKey = envnames.CliUserAccessToken case credential.TokenTypeTAT: - envKey = envvars.CliTenantAccessToken + envKey = envnames.CliTenantAccessToken default: return nil, nil } @@ -109,6 +109,13 @@ func (p *Provider) ResolveToken(ctx context.Context, req credential.TokenSpec) ( return &credential.Token{Value: token, Source: "env:" + envKey}, nil } +func parseBrand(value string) credential.Brand { + if strings.ToLower(strings.TrimSpace(value)) == "lark" { + return credential.BrandLark + } + return credential.BrandFeishu +} + func init() { credential.Register(&Provider{}) } diff --git a/extension/credential/env/env_test.go b/extension/credential/env/env_test.go index 096bd9fe29..15d069685c 100644 --- a/extension/credential/env/env_test.go +++ b/extension/credential/env/env_test.go @@ -9,8 +9,8 @@ import ( "strings" "testing" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/extension/credential" - "github.com/larksuite/cli/internal/envvars" ) func TestProvider_Name(t *testing.T) { @@ -20,9 +20,9 @@ func TestProvider_Name(t *testing.T) { } func TestResolveAccount_BothSet(t *testing.T) { - t.Setenv(envvars.CliAppID, "cli_test") - t.Setenv(envvars.CliAppSecret, "secret_test") - t.Setenv(envvars.CliBrand, " LARK ") + t.Setenv(envnames.CliAppID, "cli_test") + t.Setenv(envnames.CliAppSecret, "secret_test") + t.Setenv(envnames.CliBrand, " LARK ") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { @@ -41,7 +41,7 @@ func TestResolveAccount_NeitherSet(t *testing.T) { } func TestResolveAccount_OnlyIDSet(t *testing.T) { - t.Setenv(envvars.CliAppID, "cli_test") + t.Setenv(envnames.CliAppID, "cli_test") _, err := (&Provider{}).ResolveAccount(context.Background()) var blockErr *credential.BlockError if !errors.As(err, &blockErr) { @@ -50,8 +50,8 @@ func TestResolveAccount_OnlyIDSet(t *testing.T) { } func TestResolveAccount_AppIDAndUserTokenWithoutSecret(t *testing.T) { - t.Setenv(envvars.CliAppID, "cli_test") - t.Setenv(envvars.CliUserAccessToken, "uat_test") + t.Setenv(envnames.CliAppID, "cli_test") + t.Setenv(envnames.CliUserAccessToken, "uat_test") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { @@ -69,7 +69,7 @@ func TestResolveAccount_AppIDAndUserTokenWithoutSecret(t *testing.T) { } func TestResolveAccount_OnlySecretSet(t *testing.T) { - t.Setenv(envvars.CliAppSecret, "secret_test") + t.Setenv(envnames.CliAppSecret, "secret_test") _, err := (&Provider{}).ResolveAccount(context.Background()) var blockErr *credential.BlockError if !errors.As(err, &blockErr) { @@ -78,21 +78,21 @@ func TestResolveAccount_OnlySecretSet(t *testing.T) { } func TestResolveAccount_OnlyTokenSetWithoutAppID(t *testing.T) { - t.Setenv(envvars.CliUserAccessToken, "uat_test") + t.Setenv(envnames.CliUserAccessToken, "uat_test") _, err := (&Provider{}).ResolveAccount(context.Background()) var blockErr *credential.BlockError if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %v", err) } - if !strings.Contains(err.Error(), envvars.CliAppID) { - t.Fatalf("error = %v, want mention of %s", err, envvars.CliAppID) + if !strings.Contains(err.Error(), envnames.CliAppID) { + t.Fatalf("error = %v, want mention of %s", err, envnames.CliAppID) } } func TestResolveAccount_DefaultBrand(t *testing.T) { - t.Setenv(envvars.CliAppID, "cli_test") - t.Setenv(envvars.CliAppSecret, "secret_test") + t.Setenv(envnames.CliAppID, "cli_test") + t.Setenv(envnames.CliAppSecret, "secret_test") acct, _ := (&Provider{}).ResolveAccount(context.Background()) if acct.Brand != "feishu" { t.Errorf("expected 'feishu', got %q", acct.Brand) @@ -100,9 +100,9 @@ func TestResolveAccount_DefaultBrand(t *testing.T) { } func TestResolveAccount_DefaultAsFromEnv(t *testing.T) { - t.Setenv(envvars.CliAppID, "cli_test") - t.Setenv(envvars.CliAppSecret, "secret_test") - t.Setenv(envvars.CliDefaultAs, "user") + t.Setenv(envnames.CliAppID, "cli_test") + t.Setenv(envnames.CliAppSecret, "secret_test") + t.Setenv(envnames.CliDefaultAs, "user") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { @@ -114,23 +114,23 @@ func TestResolveAccount_DefaultAsFromEnv(t *testing.T) { } func TestResolveToken_UATSet(t *testing.T) { - t.Setenv(envvars.CliUserAccessToken, "u-env") + t.Setenv(envnames.CliUserAccessToken, "u-env") tok, err := (&Provider{}).ResolveToken(context.Background(), credential.TokenSpec{Type: credential.TokenTypeUAT}) if err != nil { t.Fatal(err) } - if tok.Value != "u-env" || tok.Source != "env:"+envvars.CliUserAccessToken { + if tok.Value != "u-env" || tok.Source != "env:"+envnames.CliUserAccessToken { t.Errorf("unexpected: %+v", tok) } } func TestResolveToken_TATSet(t *testing.T) { - t.Setenv(envvars.CliTenantAccessToken, "t-env") + t.Setenv(envnames.CliTenantAccessToken, "t-env") tok, err := (&Provider{}).ResolveToken(context.Background(), credential.TokenSpec{Type: credential.TokenTypeTAT}) if err != nil { t.Fatal(err) } - if tok.Value != "t-env" || tok.Source != "env:"+envvars.CliTenantAccessToken { + if tok.Value != "t-env" || tok.Source != "env:"+envnames.CliTenantAccessToken { t.Errorf("unexpected: %+v", tok) } } @@ -143,9 +143,9 @@ func TestResolveToken_NotSet(t *testing.T) { } func TestResolveAccount_StrictModeBot(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliStrictMode, "bot") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliStrictMode, "bot") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -156,9 +156,9 @@ func TestResolveAccount_StrictModeBot(t *testing.T) { } func TestResolveAccount_StrictModeUser(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliStrictMode, "user") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliStrictMode, "user") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -169,9 +169,9 @@ func TestResolveAccount_StrictModeUser(t *testing.T) { } func TestResolveAccount_StrictModeOff(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliStrictMode, "off") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliStrictMode, "off") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -182,9 +182,9 @@ func TestResolveAccount_StrictModeOff(t *testing.T) { } func TestResolveAccount_InferFromUATOnly(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliUserAccessToken, "u-tok") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliUserAccessToken, "u-tok") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -198,9 +198,9 @@ func TestResolveAccount_InferFromUATOnly(t *testing.T) { } func TestResolveAccount_InferFromTATOnly(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliTenantAccessToken, "t-tok") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliTenantAccessToken, "t-tok") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -214,10 +214,10 @@ func TestResolveAccount_InferFromTATOnly(t *testing.T) { } func TestResolveAccount_InferBothTokens(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliUserAccessToken, "u-tok") - t.Setenv(envvars.CliTenantAccessToken, "t-tok") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliUserAccessToken, "u-tok") + t.Setenv(envnames.CliTenantAccessToken, "t-tok") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -231,11 +231,11 @@ func TestResolveAccount_InferBothTokens(t *testing.T) { } func TestResolveAccount_StrictModeOverridesTokenInference(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliUserAccessToken, "u-tok") - t.Setenv(envvars.CliTenantAccessToken, "t-tok") - t.Setenv(envvars.CliStrictMode, "bot") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliUserAccessToken, "u-tok") + t.Setenv(envnames.CliTenantAccessToken, "t-tok") + t.Setenv(envnames.CliStrictMode, "bot") acct, err := (&Provider{}).ResolveAccount(context.Background()) if err != nil { t.Fatal(err) @@ -246,9 +246,9 @@ func TestResolveAccount_StrictModeOverridesTokenInference(t *testing.T) { } func TestResolveAccount_InvalidStrictModeRejected(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliStrictMode, "invalid") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliStrictMode, "invalid") _, err := (&Provider{}).ResolveAccount(context.Background()) if err == nil { @@ -258,15 +258,15 @@ func TestResolveAccount_InvalidStrictModeRejected(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %T", err) } - if !strings.Contains(err.Error(), envvars.CliStrictMode) { - t.Fatalf("error = %v, want mention of %s", err, envvars.CliStrictMode) + if !strings.Contains(err.Error(), envnames.CliStrictMode) { + t.Fatalf("error = %v, want mention of %s", err, envnames.CliStrictMode) } } func TestResolveAccount_InvalidDefaultAsRejected(t *testing.T) { - t.Setenv(envvars.CliAppID, "app") - t.Setenv(envvars.CliAppSecret, "secret") - t.Setenv(envvars.CliDefaultAs, "invalid") + t.Setenv(envnames.CliAppID, "app") + t.Setenv(envnames.CliAppSecret, "secret") + t.Setenv(envnames.CliDefaultAs, "invalid") _, err := (&Provider{}).ResolveAccount(context.Background()) if err == nil { @@ -276,7 +276,7 @@ func TestResolveAccount_InvalidDefaultAsRejected(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %T", err) } - if !strings.Contains(err.Error(), envvars.CliDefaultAs) { - t.Fatalf("error = %v, want mention of %s", err, envvars.CliDefaultAs) + if !strings.Contains(err.Error(), envnames.CliDefaultAs) { + t.Fatalf("error = %v, want mention of %s", err, envnames.CliDefaultAs) } } diff --git a/extension/credential/sidecar/provider.go b/extension/credential/sidecar/provider.go index d01f56616b..39a10583ed 100644 --- a/extension/credential/sidecar/provider.go +++ b/extension/credential/sidecar/provider.go @@ -14,10 +14,10 @@ import ( "context" "fmt" "os" + "strings" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/extension/credential" - "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/sidecar" ) @@ -32,7 +32,7 @@ func (p *Provider) Priority() int { return 0 } // placeholder secret, and SupportedIdentities derived from STRICT_MODE. // Returns nil, nil when sidecar mode is not active (AUTH_PROXY not set). func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, error) { - proxyAddr := os.Getenv(envvars.CliAuthProxy) + proxyAddr := os.Getenv(envnames.CliAuthProxy) if proxyAddr == "" { return nil, nil // not in sidecar mode, skip } @@ -40,26 +40,26 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err if err := sidecar.ValidateProxyAddr(proxyAddr); err != nil { return nil, &credential.BlockError{ Provider: "sidecar", - Reason: fmt.Sprintf("invalid %s %q: %v", envvars.CliAuthProxy, proxyAddr, err), + Reason: fmt.Sprintf("invalid %s %q: %v", envnames.CliAuthProxy, proxyAddr, err), } } - appID := os.Getenv(envvars.CliAppID) + appID := os.Getenv(envnames.CliAppID) if appID == "" { return nil, &credential.BlockError{ Provider: "sidecar", - Reason: envvars.CliAuthProxy + " is set but " + envvars.CliAppID + " is missing", + Reason: envnames.CliAuthProxy + " is set but " + envnames.CliAppID + " is missing", } } - if os.Getenv(envvars.CliProxyKey) == "" { + if os.Getenv(envnames.CliProxyKey) == "" { return nil, &credential.BlockError{ Provider: "sidecar", - Reason: envvars.CliAuthProxy + " is set but " + envvars.CliProxyKey + " is missing", + Reason: envnames.CliAuthProxy + " is set but " + envnames.CliProxyKey + " is missing", } } - brand := credential.Brand(core.ParseBrand(os.Getenv(envvars.CliBrand))) + brand := parseBrand(os.Getenv(envnames.CliBrand)) acct := &credential.Account{ AppID: appID, @@ -68,7 +68,7 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err } // Parse DefaultAs - switch id := credential.Identity(os.Getenv(envvars.CliDefaultAs)); id { + switch id := credential.Identity(os.Getenv(envnames.CliDefaultAs)); id { case "", credential.IdentityAuto: acct.DefaultAs = id case credential.IdentityUser, credential.IdentityBot: @@ -76,12 +76,12 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err default: return nil, &credential.BlockError{ Provider: "sidecar", - Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envvars.CliDefaultAs, id), + Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envnames.CliDefaultAs, id), } } // Parse SupportedIdentities from STRICT_MODE, default to SupportsAll. - switch strictMode := os.Getenv(envvars.CliStrictMode); strictMode { + switch strictMode := os.Getenv(envnames.CliStrictMode); strictMode { case "bot": acct.SupportedIdentities = credential.SupportsBot case "user": @@ -91,7 +91,7 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err default: return nil, &credential.BlockError{ Provider: "sidecar", - Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envvars.CliStrictMode, strictMode), + Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envnames.CliStrictMode, strictMode), } } @@ -103,7 +103,7 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err // (user vs bot), strips it, and the sidecar injects the real token. // Returns nil, nil when sidecar mode is not active. func (p *Provider) ResolveToken(ctx context.Context, req credential.TokenSpec) (*credential.Token, error) { - if os.Getenv(envvars.CliAuthProxy) == "" { + if os.Getenv(envnames.CliAuthProxy) == "" { return nil, nil } @@ -124,6 +124,13 @@ func (p *Provider) ResolveToken(ctx context.Context, req credential.TokenSpec) ( }, nil } +func parseBrand(value string) credential.Brand { + if strings.ToLower(strings.TrimSpace(value)) == "lark" { + return credential.BrandLark + } + return credential.BrandFeishu +} + func init() { credential.Register(&Provider{}) } diff --git a/extension/credential/sidecar/provider_test.go b/extension/credential/sidecar/provider_test.go index e265b5652f..7b1c6100a3 100644 --- a/extension/credential/sidecar/provider_test.go +++ b/extension/credential/sidecar/provider_test.go @@ -10,8 +10,8 @@ import ( "os" "testing" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/extension/credential" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/sidecar" ) @@ -40,7 +40,7 @@ func unsetEnv(t *testing.T, key string) { } func TestResolveAccount_NotActive(t *testing.T) { - unsetEnv(t, envvars.CliAuthProxy) + unsetEnv(t, envnames.CliAuthProxy) p := &Provider{} acct, err := p.ResolveAccount(context.Background()) @@ -53,12 +53,12 @@ func TestResolveAccount_NotActive(t *testing.T) { } func TestResolveAccount_Active(t *testing.T) { - setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") - setEnv(t, envvars.CliProxyKey, "test-key") - setEnv(t, envvars.CliAppID, "cli_test123") - setEnv(t, envvars.CliBrand, " LARK ") - unsetEnv(t, envvars.CliDefaultAs) - unsetEnv(t, envvars.CliStrictMode) + setEnv(t, envnames.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envnames.CliProxyKey, "test-key") + setEnv(t, envnames.CliAppID, "cli_test123") + setEnv(t, envnames.CliBrand, " LARK ") + unsetEnv(t, envnames.CliDefaultAs) + unsetEnv(t, envnames.CliStrictMode) p := &Provider{} acct, err := p.ResolveAccount(context.Background()) @@ -83,9 +83,9 @@ func TestResolveAccount_Active(t *testing.T) { } func TestResolveAccount_MissingProxyKey(t *testing.T) { - setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") - unsetEnv(t, envvars.CliProxyKey) - setEnv(t, envvars.CliAppID, "cli_test") + setEnv(t, envnames.CliAuthProxy, "http://127.0.0.1:16384") + unsetEnv(t, envnames.CliProxyKey) + setEnv(t, envnames.CliAppID, "cli_test") p := &Provider{} _, err := p.ResolveAccount(context.Background()) @@ -98,9 +98,9 @@ func TestResolveAccount_MissingProxyKey(t *testing.T) { } func TestResolveAccount_MissingAppID(t *testing.T) { - setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") - setEnv(t, envvars.CliProxyKey, "test-key") - unsetEnv(t, envvars.CliAppID) + setEnv(t, envnames.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envnames.CliProxyKey, "test-key") + unsetEnv(t, envnames.CliAppID) p := &Provider{} _, err := p.ResolveAccount(context.Background()) @@ -113,9 +113,9 @@ func TestResolveAccount_MissingAppID(t *testing.T) { } func TestResolveAccount_StrictMode(t *testing.T) { - setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") - setEnv(t, envvars.CliProxyKey, "test-key") - setEnv(t, envvars.CliAppID, "cli_test") + setEnv(t, envnames.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envnames.CliProxyKey, "test-key") + setEnv(t, envnames.CliAppID, "cli_test") tests := []struct { mode string @@ -131,9 +131,9 @@ func TestResolveAccount_StrictMode(t *testing.T) { for _, tt := range tests { t.Run("strict_"+tt.mode, func(t *testing.T) { if tt.mode == "" { - unsetEnv(t, envvars.CliStrictMode) + unsetEnv(t, envnames.CliStrictMode) } else { - setEnv(t, envvars.CliStrictMode, tt.mode) + setEnv(t, envnames.CliStrictMode, tt.mode) } acct, err := p.ResolveAccount(context.Background()) if err != nil { @@ -147,7 +147,7 @@ func TestResolveAccount_StrictMode(t *testing.T) { } func TestResolveToken_NotActive(t *testing.T) { - unsetEnv(t, envvars.CliAuthProxy) + unsetEnv(t, envnames.CliAuthProxy) p := &Provider{} tok, err := p.ResolveToken(context.Background(), credential.TokenSpec{Type: credential.TokenTypeUAT}) @@ -160,8 +160,8 @@ func TestResolveToken_NotActive(t *testing.T) { } func TestResolveToken_Sentinels(t *testing.T) { - setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") - setEnv(t, envvars.CliProxyKey, "test-key") + setEnv(t, envnames.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envnames.CliProxyKey, "test-key") p := &Provider{} diff --git a/extension/transport/sidecar/interceptor.go b/extension/transport/sidecar/interceptor.go index 6d928bd8a7..339dcf1d1e 100644 --- a/extension/transport/sidecar/interceptor.go +++ b/extension/transport/sidecar/interceptor.go @@ -20,8 +20,8 @@ import ( "os" "strings" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/extension/transport" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/sidecar" ) @@ -36,15 +36,15 @@ func (p *Provider) Name() string { return "sidecar" } // the non-sidecar transport path (where the credential layer will typically // block them for lack of a valid account). func (p *Provider) ResolveInterceptor(ctx context.Context) transport.Interceptor { - proxyAddr := os.Getenv(envvars.CliAuthProxy) + proxyAddr := os.Getenv(envnames.CliAuthProxy) if proxyAddr == "" { return nil } if err := sidecar.ValidateProxyAddr(proxyAddr); err != nil { - fmt.Fprintf(os.Stderr, "WARNING: invalid %s, sidecar interceptor disabled: %v\n", envvars.CliAuthProxy, err) + fmt.Fprintf(os.Stderr, "WARNING: invalid %s, sidecar interceptor disabled: %v\n", envnames.CliAuthProxy, err) return nil } - key := os.Getenv(envvars.CliProxyKey) + key := os.Getenv(envnames.CliProxyKey) return &Interceptor{ key: []byte(key), sidecarHost: sidecar.ProxyHost(proxyAddr), @@ -166,12 +166,12 @@ func detectSentinel(req *http.Request) (identity, authHeader string) { } func init() { - proxyAddr := os.Getenv(envvars.CliAuthProxy) + proxyAddr := os.Getenv(envnames.CliAuthProxy) if proxyAddr == "" { return } if err := sidecar.ValidateProxyAddr(proxyAddr); err != nil { - fmt.Fprintf(os.Stderr, "WARNING: ignoring invalid %s: %v\n", envvars.CliAuthProxy, err) + fmt.Fprintf(os.Stderr, "WARNING: ignoring invalid %s: %v\n", envnames.CliAuthProxy, err) return } transport.Register(&Provider{}) diff --git a/internal/cmdutil/factory_default_test.go b/internal/cmdutil/factory_default_test.go index 1f47971ceb..94f9438128 100644 --- a/internal/cmdutil/factory_default_test.go +++ b/internal/cmdutil/factory_default_test.go @@ -8,12 +8,12 @@ import ( "errors" "testing" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/errs" _ "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/vfs/localfileio" ) @@ -29,10 +29,10 @@ func (p *countingFileIOProvider) ResolveFileIO(context.Context) fileio.FileIO { } func TestNewDefault_InvocationProfileUsedByStrictModeAndConfig(t *testing.T) { - t.Setenv(envvars.CliAppID, "") - t.Setenv(envvars.CliAppSecret, "") - t.Setenv(envvars.CliUserAccessToken, "") - t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envnames.CliAppID, "") + t.Setenv(envnames.CliAppSecret, "") + t.Setenv(envnames.CliUserAccessToken, "") + t.Setenv(envnames.CliTenantAccessToken, "") dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) @@ -77,10 +77,10 @@ func TestNewDefault_InvocationProfileUsedByStrictModeAndConfig(t *testing.T) { } func TestNewDefault_InvocationProfileMissingSticksAcrossEarlyStrictMode(t *testing.T) { - t.Setenv(envvars.CliAppID, "") - t.Setenv(envvars.CliAppSecret, "") - t.Setenv(envvars.CliUserAccessToken, "") - t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envnames.CliAppID, "") + t.Setenv(envnames.CliAppSecret, "") + t.Setenv(envnames.CliUserAccessToken, "") + t.Setenv(envnames.CliTenantAccessToken, "") dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) @@ -118,11 +118,11 @@ func TestNewDefault_InvocationProfileMissingSticksAcrossEarlyStrictMode(t *testi } func TestNewDefault_ResolveAs_UsesDefaultAsFromEnvAccount(t *testing.T) { - t.Setenv(envvars.CliAppID, "env-app") - t.Setenv(envvars.CliAppSecret, "env-secret") - t.Setenv(envvars.CliDefaultAs, "user") - t.Setenv(envvars.CliUserAccessToken, "") - t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envnames.CliAppID, "env-app") + t.Setenv(envnames.CliAppSecret, "env-secret") + t.Setenv(envnames.CliDefaultAs, "user") + t.Setenv(envnames.CliUserAccessToken, "") + t.Setenv(envnames.CliTenantAccessToken, "") t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f := NewDefault(nil, InvocationContext{}) @@ -138,11 +138,11 @@ func TestNewDefault_ResolveAs_UsesDefaultAsFromEnvAccount(t *testing.T) { } func TestNewDefault_ConfigReturnsCliConfigCopyOfCredentialAccount(t *testing.T) { - t.Setenv(envvars.CliAppID, "env-app") - t.Setenv(envvars.CliAppSecret, "env-secret") - t.Setenv(envvars.CliDefaultAs, "") - t.Setenv(envvars.CliUserAccessToken, "uat-token") - t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envnames.CliAppID, "env-app") + t.Setenv(envnames.CliAppSecret, "env-secret") + t.Setenv(envnames.CliDefaultAs, "") + t.Setenv(envnames.CliUserAccessToken, "uat-token") + t.Setenv(envnames.CliTenantAccessToken, "") t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f := NewDefault(nil, InvocationContext{}) @@ -163,11 +163,11 @@ func TestNewDefault_ConfigReturnsCliConfigCopyOfCredentialAccount(t *testing.T) } func TestNewDefault_ConfigUsesRuntimePlaceholderForTokenOnlyEnvAccount(t *testing.T) { - t.Setenv(envvars.CliAppID, "env-app") - t.Setenv(envvars.CliAppSecret, "") - t.Setenv(envvars.CliDefaultAs, "") - t.Setenv(envvars.CliUserAccessToken, "uat-token") - t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envnames.CliAppID, "env-app") + t.Setenv(envnames.CliAppSecret, "") + t.Setenv(envnames.CliDefaultAs, "") + t.Setenv(envnames.CliUserAccessToken, "uat-token") + t.Setenv(envnames.CliTenantAccessToken, "") t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f := NewDefault(nil, InvocationContext{}) diff --git a/internal/cmdutil/factory_proxy_warn_test.go b/internal/cmdutil/factory_proxy_warn_test.go index ffdeb488fc..114e26303e 100644 --- a/internal/cmdutil/factory_proxy_warn_test.go +++ b/internal/cmdutil/factory_proxy_warn_test.go @@ -7,8 +7,8 @@ import ( "io" "testing" + "github.com/larksuite/cli/envnames" _ "github.com/larksuite/cli/extension/credential/env" // registers the env-backed account provider - "github.com/larksuite/cli/internal/envvars" ) // installProxyWarnSpy replaces warnIfProxied with a counter for one test and @@ -61,11 +61,11 @@ func TestCachedHttpClientFunc_ProxyWarnGate(t *testing.T) { func TestCachedLarkClientFunc_ProxyWarnGate(t *testing.T) { for _, tc := range proxyWarnGateCases { t.Run(tc.name, func(t *testing.T) { - t.Setenv(envvars.CliAppID, "env-app") - t.Setenv(envvars.CliAppSecret, "env-secret") - t.Setenv(envvars.CliDefaultAs, "") - t.Setenv(envvars.CliUserAccessToken, "") - t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envnames.CliAppID, "env-app") + t.Setenv(envnames.CliAppSecret, "env-secret") + t.Setenv(envnames.CliDefaultAs, "") + t.Setenv(envnames.CliUserAccessToken, "") + t.Setenv(envnames.CliTenantAccessToken, "") t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) calls := installProxyWarnSpy(t) diff --git a/internal/cmdutil/factory_test.go b/internal/cmdutil/factory_test.go index 97eed80975..829c8722fc 100644 --- a/internal/cmdutil/factory_test.go +++ b/internal/cmdutil/factory_test.go @@ -11,11 +11,11 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/errs" extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/output" ) @@ -92,7 +92,7 @@ func TestResolveAs_DefaultAs_FromConfig(t *testing.T) { } func TestResolveAs_DefaultAs_EnvDoesNotBypassConfigSource(t *testing.T) { - t.Setenv(envvars.CliDefaultAs, "user") + t.Setenv(envnames.CliDefaultAs, "user") f, _, _, _ := TestFactory(t, &core.CliConfig{AppID: "a", AppSecret: "s"}) cmd := newCmdWithAsFlag("auto", false) diff --git a/internal/credential/integration_test.go b/internal/credential/integration_test.go index de173d1948..e462cf6232 100644 --- a/internal/credential/integration_test.go +++ b/internal/credential/integration_test.go @@ -7,11 +7,11 @@ import ( "context" "testing" + "github.com/larksuite/cli/envnames" extcred "github.com/larksuite/cli/extension/credential" envprovider "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/i18n" "github.com/larksuite/cli/internal/keychain" ) @@ -23,9 +23,9 @@ func (n *noopKC) Set(service, account, value string) error { return nil } func (n *noopKC) Remove(service, account string) error { return nil } func TestFullChain_EnvWins(t *testing.T) { - t.Setenv(envvars.CliAppID, "env_app") - t.Setenv(envvars.CliAppSecret, "env_secret") - t.Setenv(envvars.CliUserAccessToken, "env_uat") + t.Setenv(envnames.CliAppID, "env_app") + t.Setenv(envnames.CliAppSecret, "env_secret") + t.Setenv(envnames.CliUserAccessToken, "env_uat") ep := &envprovider.Provider{} cp := credential.NewCredentialProvider( @@ -82,8 +82,8 @@ func (m *mockDefaultTokenProvider) ResolveToken(ctx context.Context, req credent } func TestFullChain_ConfigStrictMode(t *testing.T) { - t.Setenv(envvars.CliAppID, "") - t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envnames.CliAppID, "") + t.Setenv(envnames.CliAppSecret, "") dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) @@ -125,8 +125,8 @@ func TestFullChain_ConfigStrictMode(t *testing.T) { // consumers (mail signature, etc.) silently fall back to defaults — defeating // the whole point of persisting --lang. func TestFullChain_LangSurvivesProductionPath(t *testing.T) { - t.Setenv(envvars.CliAppID, "") - t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envnames.CliAppID, "") + t.Setenv(envnames.CliAppSecret, "") t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) multi := &core.MultiAppConfig{ diff --git a/internal/envvars/envvars.go b/internal/envvars/envvars.go index 36b60e42d5..fc90a52f15 100644 --- a/internal/envvars/envvars.go +++ b/internal/envvars/envvars.go @@ -4,22 +4,9 @@ package envvars const ( - CliAppID = "LARKSUITE_CLI_APP_ID" - CliAppSecret = "LARKSUITE_CLI_APP_SECRET" - CliBrand = "LARKSUITE_CLI_BRAND" - CliUserAccessToken = "LARKSUITE_CLI_USER_ACCESS_TOKEN" - CliTenantAccessToken = "LARKSUITE_CLI_TENANT_ACCESS_TOKEN" - CliDefaultAs = "LARKSUITE_CLI_DEFAULT_AS" - CliStrictMode = "LARKSUITE_CLI_STRICT_MODE" - - // Sidecar proxy (auth proxy mode) - CliAuthProxy = "LARKSUITE_CLI_AUTH_PROXY" // sidecar HTTP address, e.g. "http://127.0.0.1:16384" - CliProxyKey = "LARKSUITE_CLI_PROXY_KEY" // HMAC signing key shared with sidecar - // Content safety scanning mode CliContentSafetyMode = "LARKSUITE_CLI_CONTENT_SAFETY_MODE" - CliAgentName = "LARKSUITE_CLI_AGENT_NAME" CliAgentTrace = "LARKSUITE_CLI_AGENT_TRACE" CliProxyEnable = "LARKSUITE_CLI_PROXY_ENABLE" diff --git a/internal/envvars/read.go b/internal/envvars/read.go index 34868386cd..e2a9d52315 100644 --- a/internal/envvars/read.go +++ b/internal/envvars/read.go @@ -10,12 +10,13 @@ import ( ) const ( + agentNameEnv = "LARKSUITE_CLI_AGENT_NAME" agentNameMaxLen = 128 agentTraceMaxLen = 1024 ) func AgentName() string { - return sanitizeSingleLine(os.Getenv(CliAgentName), agentNameMaxLen) + return sanitizeSingleLine(os.Getenv(agentNameEnv), agentNameMaxLen) } func AgentTrace() string { diff --git a/internal/envvars/read_test.go b/internal/envvars/read_test.go index 39903e17bf..a28277ad92 100644 --- a/internal/envvars/read_test.go +++ b/internal/envvars/read_test.go @@ -9,35 +9,35 @@ import ( ) func TestAgentName_EmptyWhenEnvUnset(t *testing.T) { - t.Setenv(CliAgentName, "") + t.Setenv(agentNameEnv, "") if got := AgentName(); got != "" { t.Fatalf("AgentName() = %q, want empty when env unset", got) } } func TestAgentName_ReturnsCleanValue(t *testing.T) { - t.Setenv(CliAgentName, "claude-code") + t.Setenv(agentNameEnv, "claude-code") if got := AgentName(); got != "claude-code" { t.Fatalf("AgentName() = %q, want %q", got, "claude-code") } } func TestAgentName_TrimsWhitespace(t *testing.T) { - t.Setenv(CliAgentName, " cursor ") + t.Setenv(agentNameEnv, " cursor ") if got := AgentName(); got != "cursor" { t.Fatalf("AgentName() = %q, want %q (whitespace trimmed)", got, "cursor") } } func TestAgentName_RejectsCRLFInjection(t *testing.T) { - t.Setenv(CliAgentName, "agent\r\nX-Evil: attack") + t.Setenv(agentNameEnv, "agent\r\nX-Evil: attack") if got := AgentName(); got != "" { t.Fatalf("AgentName() = %q, want empty for CR/LF value", got) } } func TestAgentName_RejectsControlChar(t *testing.T) { - t.Setenv(CliAgentName, "agent\x01injected") + t.Setenv(agentNameEnv, "agent\x01injected") if got := AgentName(); got != "" { t.Fatalf("AgentName() = %q, want empty for control char value", got) } @@ -45,7 +45,7 @@ func TestAgentName_RejectsControlChar(t *testing.T) { func TestAgentName_RejectsOverlongValue(t *testing.T) { longVal := strings.Repeat("a", agentNameMaxLen+1) - t.Setenv(CliAgentName, longVal) + t.Setenv(agentNameEnv, longVal) if got := AgentName(); got != "" { t.Fatalf("AgentName() returned non-empty for %d-byte value (max %d)", len(longVal), agentNameMaxLen) } diff --git a/internal/qualitygate/deptest/layering-edges.txt b/internal/qualitygate/deptest/layering-edges.txt index 5301b6aaa2..4bf0bc39e0 100644 --- a/internal/qualitygate/deptest/layering-edges.txt +++ b/internal/qualitygate/deptest/layering-edges.txt @@ -1,21 +1,4 @@ # from denied owner reason added_at -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/charcheck arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/core arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/i18n arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/keychain arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/validate arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/vfs arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/env github.com/larksuite/cli/internal/vfs/localfileio arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/charcheck arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/core arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/i18n arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/keychain arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/validate arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/vfs arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/credential/sidecar github.com/larksuite/cli/internal/vfs/localfileio arch-migration extension pre-module-split (via core/envvars) 2026-07-24 -github.com/larksuite/cli/extension/transport/sidecar github.com/larksuite/cli/internal/envvars arch-migration extension pre-module-split (via core/envvars) 2026-07-24 github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events root aggregates the existing im conversion dependency 2026-07-24 github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/common arch-migration events root aggregates the existing im conversion dependency 2026-07-24 github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events->shortcuts inversion via convert_lib 2026-07-24 diff --git a/main_noauthsidecar.go b/main_noauthsidecar.go index 514afda63d..28baa22972 100644 --- a/main_noauthsidecar.go +++ b/main_noauthsidecar.go @@ -21,7 +21,7 @@ import ( "io" "os" - "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/envnames" ) func init() { @@ -35,7 +35,7 @@ func init() { // isolation that this binary cannot provide. Factored out from init() so // tests can exercise the decision without actually calling os.Exit. func checkNoAuthsidecarBuild(getenv func(string) string, stderr io.Writer) int { - v := getenv(envvars.CliAuthProxy) + v := getenv(envnames.CliAuthProxy) if v == "" { return 0 } @@ -49,6 +49,6 @@ func checkNoAuthsidecarBuild(getenv func(string) string, stderr io.Writer) int { "To fix, either:\n"+ " - rebuild the CLI with: go build -tags authsidecar\n"+ " - or unset %s if sidecar isolation is not required\n", - envvars.CliAuthProxy, envvars.CliAuthProxy) + envnames.CliAuthProxy, envnames.CliAuthProxy) return 2 } diff --git a/main_noauthsidecar_test.go b/main_noauthsidecar_test.go index 5879015c25..7747aa9a49 100644 --- a/main_noauthsidecar_test.go +++ b/main_noauthsidecar_test.go @@ -10,7 +10,7 @@ import ( "strings" "testing" - "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/envnames" ) func TestCheckNoAuthsidecarBuild_Unset(t *testing.T) { @@ -30,7 +30,7 @@ func TestCheckNoAuthsidecarBuild_Unset(t *testing.T) { func TestCheckNoAuthsidecarBuild_Set(t *testing.T) { var stderr bytes.Buffer env := func(k string) string { - if k == envvars.CliAuthProxy { + if k == envnames.CliAuthProxy { return "http://127.0.0.1:16384" } return "" @@ -41,7 +41,7 @@ func TestCheckNoAuthsidecarBuild_Set(t *testing.T) { } msg := stderr.String() for _, want := range []string{ - envvars.CliAuthProxy, + envnames.CliAuthProxy, "authsidecar", // build-tag name must appear so operators can act on it "rebuild", } { diff --git a/sidecar/server-demo/handler_test.go b/sidecar/server-demo/handler_test.go index edd2446fef..25ce6c3552 100644 --- a/sidecar/server-demo/handler_test.go +++ b/sidecar/server-demo/handler_test.go @@ -17,10 +17,10 @@ import ( "strings" "testing" + "github.com/larksuite/cli/envnames" extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/sidecar" ) @@ -410,13 +410,13 @@ func TestProxyHandler_AcceptsAllowedAuthHeaders(t *testing.T) { } func TestRun_RejectsSelfProxy(t *testing.T) { - old, had := os.LookupEnv(envvars.CliAuthProxy) - os.Setenv(envvars.CliAuthProxy, "http://127.0.0.1:16384") + old, had := os.LookupEnv(envnames.CliAuthProxy) + os.Setenv(envnames.CliAuthProxy, "http://127.0.0.1:16384") defer func() { if had { - os.Setenv(envvars.CliAuthProxy, old) + os.Setenv(envnames.CliAuthProxy, old) } else { - os.Unsetenv(envvars.CliAuthProxy) + os.Unsetenv(envnames.CliAuthProxy) } }() @@ -424,8 +424,8 @@ func TestRun_RejectsSelfProxy(t *testing.T) { if err == nil { t.Fatal("expected error when AUTH_PROXY is set") } - if !strings.Contains(err.Error(), envvars.CliAuthProxy) { - t.Errorf("error should mention %s, got: %v", envvars.CliAuthProxy, err) + if !strings.Contains(err.Error(), envnames.CliAuthProxy) { + t.Errorf("error should mention %s, got: %v", envnames.CliAuthProxy, err) } } diff --git a/sidecar/server-demo/main.go b/sidecar/server-demo/main.go index 7292daa263..1005655feb 100644 --- a/sidecar/server-demo/main.go +++ b/sidecar/server-demo/main.go @@ -28,9 +28,9 @@ import ( "syscall" "time" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/vfs" "github.com/larksuite/cli/sidecar" ) @@ -62,8 +62,8 @@ func run(ctx context.Context, listen, keyFile, logFile, profile string) error { // Reject self-proxy: if this process inherited AUTH_PROXY, the sidecar // credential provider would activate and return sentinel tokens instead // of real ones, breaking the "trusted side holds real credentials" premise. - if v := os.Getenv(envvars.CliAuthProxy); v != "" { - return fmt.Errorf("%s is set in this environment (%s); unset it before starting the sidecar server", envvars.CliAuthProxy, v) + if v := os.Getenv(envnames.CliAuthProxy); v != "" { + return fmt.Errorf("%s is set in this environment (%s); unset it before starting the sidecar server", envnames.CliAuthProxy, v) } if listen == "" { return fmt.Errorf("invalid --listen address: empty") @@ -155,10 +155,10 @@ func run(ctx context.Context, listen, keyFile, logFile, profile string) error { fmt.Fprintf(os.Stderr, "HMAC key prefix: %s\n", keyPrefix) fmt.Fprintf(os.Stderr, "Full key written to %s (mode 0600)\n", keyFile) fmt.Fprintf(os.Stderr, "\nSet in sandbox:\n") - fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliAuthProxy, proxyURL) - fmt.Fprintf(os.Stderr, " export %s=\"\"\n", envvars.CliProxyKey, keyFile) - fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliAppID, cfg.AppID) - fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliBrand, string(cfg.Brand)) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envnames.CliAuthProxy, proxyURL) + fmt.Fprintf(os.Stderr, " export %s=\"\"\n", envnames.CliProxyKey, keyFile) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envnames.CliAppID, cfg.AppID) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envnames.CliBrand, string(cfg.Brand)) if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { return fmt.Errorf("sidecar server exited unexpectedly: %v", err) diff --git a/sidecar/server-multi-tenant-demo/handler_test.go b/sidecar/server-multi-tenant-demo/handler_test.go index ffdd64e68d..10dd9d0f0c 100644 --- a/sidecar/server-multi-tenant-demo/handler_test.go +++ b/sidecar/server-multi-tenant-demo/handler_test.go @@ -19,10 +19,10 @@ import ( "strings" "testing" + "github.com/larksuite/cli/envnames" extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/sidecar" ) @@ -412,7 +412,7 @@ func TestProxyHandler_AcceptsAllowedAuthHeaders(t *testing.T) { } func TestRun_RejectsSelfProxy(t *testing.T) { - t.Setenv(envvars.CliAuthProxy, "http://127.0.0.1:16384") + t.Setenv(envnames.CliAuthProxy, "http://127.0.0.1:16384") t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) keyPath := filepath.Join(t.TempDir(), "proxy.key") @@ -420,8 +420,8 @@ func TestRun_RejectsSelfProxy(t *testing.T) { if err == nil { t.Fatal("expected error when AUTH_PROXY is set") } - if !strings.Contains(err.Error(), envvars.CliAuthProxy) { - t.Errorf("error should mention %s, got: %v", envvars.CliAuthProxy, err) + if !strings.Contains(err.Error(), envnames.CliAuthProxy) { + t.Errorf("error should mention %s, got: %v", envnames.CliAuthProxy, err) } } diff --git a/sidecar/server-multi-tenant-demo/main.go b/sidecar/server-multi-tenant-demo/main.go index 42b1551d77..0ea9f0757a 100644 --- a/sidecar/server-multi-tenant-demo/main.go +++ b/sidecar/server-multi-tenant-demo/main.go @@ -29,9 +29,9 @@ import ( "syscall" "time" + "github.com/larksuite/cli/envnames" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" "github.com/larksuite/cli/sidecar" @@ -62,8 +62,8 @@ func defaultKeyFile() string { } func run(ctx context.Context, listen, keyFile, keysDir, logFile, profile string) error { - if v := os.Getenv(envvars.CliAuthProxy); v != "" { - return fmt.Errorf("%s is set in this environment (%s); unset it before starting the sidecar server", envvars.CliAuthProxy, v) + if v := os.Getenv(envnames.CliAuthProxy); v != "" { + return fmt.Errorf("%s is set in this environment (%s); unset it before starting the sidecar server", envnames.CliAuthProxy, v) } if listen == "" { return fmt.Errorf("invalid --listen address: empty") @@ -183,10 +183,10 @@ func run(ctx context.Context, listen, keyFile, keysDir, logFile, profile string) fmt.Fprintf(os.Stderr, "Full key written to %s (mode 0600)\n", keyFile) fmt.Fprintf(os.Stderr, "Client keys dir: %s\n", keysDir) fmt.Fprintf(os.Stderr, "\nSet in sandbox:\n") - fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliAuthProxy, proxyURL) - fmt.Fprintf(os.Stderr, " export %s=\"\"\n", envvars.CliProxyKey, keyFile) - fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliAppID, cfg.AppID) - fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliBrand, string(cfg.Brand)) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envnames.CliAuthProxy, proxyURL) + fmt.Fprintf(os.Stderr, " export %s=\"\"\n", envnames.CliProxyKey, keyFile) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envnames.CliAppID, cfg.AppID) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envnames.CliBrand, string(cfg.Brand)) if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { return fmt.Errorf("sidecar server exited unexpectedly: %v", err) From d62f8dcbe82624f9d575e0df6854cf862bec1b5c Mon Sep 17 00:00:00 2001 From: shanglei Date: Sat, 25 Jul 2026 17:05:01 +0800 Subject: [PATCH 14/50] refactor(events): move message conversion below shortcuts --- events/im/message_receive.go | 8 +- .../imcontent}/card.go | 4 +- .../imcontent}/card_test.go | 2 +- .../imcontent}/card_userdsl.go | 4 +- .../imcontent}/card_userdsl_test.go | 2 +- internal/imcontent/convert.go | 93 +++++++++++++++++ internal/imcontent/helpers.go | 99 +++++++++++++++++++ .../imcontent}/media.go | 2 +- .../imcontent}/misc.go | 10 +- .../imcontent}/text.go | 7 +- .../imcontent}/text_test.go | 2 +- .../qualitygate/deptest/layering-edges.txt | 4 - shortcuts/im/convert_lib/content_convert.go | 46 +-------- .../im/convert_lib/content_media_misc_test.go | 73 +++++++------- shortcuts/im/convert_lib/helpers.go | 80 ++------------- shortcuts/im/convert_lib/merge.go | 21 +--- shortcuts/im/convert_lib/pure_adapters.go | 27 +++++ 17 files changed, 292 insertions(+), 192 deletions(-) rename {shortcuts/im/convert_lib => internal/imcontent}/card.go (99%) rename {shortcuts/im/convert_lib => internal/imcontent}/card_test.go (99%) rename {shortcuts/im/convert_lib => internal/imcontent}/card_userdsl.go (99%) rename {shortcuts/im/convert_lib => internal/imcontent}/card_userdsl_test.go (99%) create mode 100644 internal/imcontent/convert.go create mode 100644 internal/imcontent/helpers.go rename {shortcuts/im/convert_lib => internal/imcontent}/media.go (99%) rename {shortcuts/im/convert_lib => internal/imcontent}/misc.go (97%) rename {shortcuts/im/convert_lib => internal/imcontent}/text.go (96%) rename {shortcuts/im/convert_lib => internal/imcontent}/text_test.go (99%) create mode 100644 shortcuts/im/convert_lib/pure_adapters.go diff --git a/events/im/message_receive.go b/events/im/message_receive.go index ae73021232..4d9ae4160a 100644 --- a/events/im/message_receive.go +++ b/events/im/message_receive.go @@ -8,7 +8,7 @@ import ( "encoding/json" "github.com/larksuite/cli/internal/event" - convertlib "github.com/larksuite/cli/shortcuts/im/convert_lib" + "github.com/larksuite/cli/internal/imcontent" ) // ImMessageReceiveOutput is the flattened shape for im.message.receive_v1; `desc` tags drive the reflected schema. @@ -74,11 +74,11 @@ func processImMessageReceive(_ context.Context, _ event.APIClient, raw *event.Ra msg := envelope.Event.Message var content string if msg.MessageType == "interactive" { - content = convertlib.ConvertInteractiveEventContent(msg.Content, msg.Mentions) + content = imcontent.ConvertInteractiveEventContent(msg.Content, msg.Mentions) } else { - content = convertlib.ConvertBodyContent(msg.MessageType, &convertlib.ConvertContext{ + content = imcontent.ConvertBodyContent(msg.MessageType, &imcontent.ConvertContext{ RawContent: msg.Content, - MentionMap: convertlib.BuildMentionKeyMap(msg.Mentions), + MentionMap: imcontent.BuildMentionKeyMap(msg.Mentions), }) } diff --git a/shortcuts/im/convert_lib/card.go b/internal/imcontent/card.go similarity index 99% rename from shortcuts/im/convert_lib/card.go rename to internal/imcontent/card.go index e58bf49595..635fb84e21 100644 --- a/shortcuts/im/convert_lib/card.go +++ b/internal/imcontent/card.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import ( "encoding/json" @@ -1639,7 +1639,7 @@ func (c *cardConverter) convertAt(prop cardObj) string { if name, _ := mention["name"].(string); name != "" { userName = name } - if id := extractMentionOpenId(mention["id"]); id != "" { + if id := extractMentionOpenID(mention["id"]); id != "" { actualUserID = id fromMentions = true } diff --git a/shortcuts/im/convert_lib/card_test.go b/internal/imcontent/card_test.go similarity index 99% rename from shortcuts/im/convert_lib/card_test.go rename to internal/imcontent/card_test.go index 1c25486eaf..1f41f8eadd 100644 --- a/shortcuts/im/convert_lib/card_test.go +++ b/internal/imcontent/card_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import ( "strings" diff --git a/shortcuts/im/convert_lib/card_userdsl.go b/internal/imcontent/card_userdsl.go similarity index 99% rename from shortcuts/im/convert_lib/card_userdsl.go rename to internal/imcontent/card_userdsl.go index 62a0168079..bbf7e3bb77 100644 --- a/shortcuts/im/convert_lib/card_userdsl.go +++ b/internal/imcontent/card_userdsl.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import ( "encoding/json" @@ -39,7 +39,7 @@ func buildMentionAtMap(mentions []interface{}) map[string]string { item, _ := raw.(map[string]interface{}) key, _ := item["key"].(string) name, _ := item["name"].(string) - openID := extractMentionOpenId(item["id"]) + openID := extractMentionOpenID(item["id"]) if key == "" { continue } diff --git a/shortcuts/im/convert_lib/card_userdsl_test.go b/internal/imcontent/card_userdsl_test.go similarity index 99% rename from shortcuts/im/convert_lib/card_userdsl_test.go rename to internal/imcontent/card_userdsl_test.go index 6f73ea3f86..7e1f0f2f7f 100644 --- a/shortcuts/im/convert_lib/card_userdsl_test.go +++ b/internal/imcontent/card_userdsl_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import ( "encoding/json" diff --git a/internal/imcontent/convert.go b/internal/imcontent/convert.go new file mode 100644 index 0000000000..25a688d43a --- /dev/null +++ b/internal/imcontent/convert.go @@ -0,0 +1,93 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package imcontent converts Lark message content into human-readable text. +package imcontent + +import "fmt" + +// ContentConverter converts one message type's raw content. +type ContentConverter interface { + Convert(ctx *ConvertContext) string +} + +// ConvertContext contains the data needed by pure message conversion. +type ConvertContext struct { + RawContent string + MentionMap map[string]string + Mentions []interface{} +} + +var converters = map[string]ContentConverter{ + "text": textConverter{}, + "post": postConverter{}, + "image": imageConverter{}, + "file": fileConverter{}, + "audio": audioMsgConverter{}, + "video": videoMsgConverter{}, + "media": videoMsgConverter{}, + "sticker": stickerConverter{}, + "interactive": interactiveConverter{}, + "share_chat": shareChatConverter{}, + "share_user": shareUserConverter{}, + "location": locationConverter{}, + "merge_forward": mergeForwardConverter{}, + "folder": folderConverter{}, + "share_calendar_event": calendarEventConverter{}, + "calendar": calendarInviteConverter{}, + "general_calendar": generalCalendarConverter{}, + "video_chat": videoChatConverter{}, + "system": systemConverter{}, + "todo": todoConverter{}, + "vote": voteConverter{}, + "hongbao": hongbaoConverter{}, +} + +// ConvertBodyContent converts a raw message body to human-readable text. +func ConvertBodyContent(messageType string, ctx *ConvertContext) string { + if ctx == nil || ctx.RawContent == "" { + return "" + } + if converter, ok := converters[messageType]; ok { + return converter.Convert(ctx) + } + return fmt.Sprintf("[%s]", messageType) +} + +type mergeForwardConverter struct{} + +func (mergeForwardConverter) Convert(ctx *ConvertContext) string { + ids := ParseMergeForwardIDs(ctx.RawContent) + if len(ids) > 0 { + return fmt.Sprintf("[Merged forward: %d messages]", len(ids)) + } + return "[Merged forward]" +} + +// ParseMergeForwardIDs extracts message IDs from merge_forward content. +func ParseMergeForwardIDs(raw string) []string { + parsed, err := ParseJSONObject(raw) + if err != nil { + return nil + } + rawIDs, _ := parsed["create_message_ids"].([]interface{}) + ids := make([]string, 0, len(rawIDs)) + for _, rawID := range rawIDs { + if id, ok := rawID.(string); ok { + ids = append(ids, id) + } + } + return ids +} + +func extractMentionOpenID(id interface{}) string { + switch value := id.(type) { + case string: + return value + case map[string]interface{}: + openID, _ := value["open_id"].(string) + return openID + default: + return "" + } +} diff --git a/internal/imcontent/helpers.go b/internal/imcontent/helpers.go new file mode 100644 index 0000000000..7ae06d271c --- /dev/null +++ b/internal/imcontent/helpers.go @@ -0,0 +1,99 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package imcontent + +import ( + "encoding/json" + "fmt" + "strconv" + "strings" + "time" +) + +// ParseJSONObject parses a raw JSON string into a map. +func ParseJSONObject(raw string) (map[string]interface{}, error) { + var value map[string]interface{} + if err := json.Unmarshal([]byte(raw), &value); err != nil { + return nil, err + } + return value, nil +} + +func invalidJSONPlaceholder(kind string) string { + if kind == "" { + return "[Invalid JSON content]" + } + return fmt.Sprintf("[Invalid %s JSON]", kind) +} + +// BuildMentionKeyMap builds a key-to-name lookup from a message mentions array. +func BuildMentionKeyMap(mentions []interface{}) map[string]string { + result := map[string]string{} + for _, raw := range mentions { + item, _ := raw.(map[string]interface{}) + key, _ := item["key"].(string) + name, _ := item["name"].(string) + if key != "" && name != "" { + result[key] = name + } + } + return result +} + +// ResolveMentionKeys replaces mention keys in text with @name format. +func ResolveMentionKeys(text string, mentionMap map[string]string) string { + for key, name := range mentionMap { + text = strings.ReplaceAll(text, key, "@"+name) + } + return text +} + +// FormatTimestamp converts a Unix timestamp string in seconds or milliseconds +// to local time. It returns an empty string for empty or invalid values. +func FormatTimestamp(timestamp string) string { + if timestamp == "" { + return "" + } + value, err := strconv.ParseInt(timestamp, 10, 64) + if err != nil || value == 0 { + return "" + } + if len(strings.TrimLeft(timestamp, "+-")) >= 13 { + value /= 1000 + } + return time.Unix(value, 0).Local().Format("2006-01-02 15:04:05") +} + +var xmlBodyEscaper = strings.NewReplacer( + "&", "&", + "<", "<", + ">", ">", +) + +func xmlEscapeBody(value string) string { + return xmlBodyEscaper.Replace(value) +} + +func escapeMDLinkText(value string) string { + value = strings.ReplaceAll(value, `[`, `\[`) + value = strings.ReplaceAll(value, `]`, `\]`) + return value +} + +// ExtractPostBlocksText extracts plain text from post-style content blocks. +func ExtractPostBlocksText(blocks []interface{}) string { + var lines []string + for _, paragraph := range blocks { + elements, _ := paragraph.([]interface{}) + var builder strings.Builder + for _, raw := range elements { + element, _ := raw.(map[string]interface{}) + builder.WriteString(renderPostElem(element)) + } + if line := builder.String(); line != "" { + lines = append(lines, line) + } + } + return strings.Join(lines, "\n") +} diff --git a/shortcuts/im/convert_lib/media.go b/internal/imcontent/media.go similarity index 99% rename from shortcuts/im/convert_lib/media.go rename to internal/imcontent/media.go index 2bac70dc45..bbb295cf50 100644 --- a/shortcuts/im/convert_lib/media.go +++ b/internal/imcontent/media.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import "fmt" diff --git a/shortcuts/im/convert_lib/misc.go b/internal/imcontent/misc.go similarity index 97% rename from shortcuts/im/convert_lib/misc.go rename to internal/imcontent/misc.go index 9ee9c53f20..0fb8173cc5 100644 --- a/shortcuts/im/convert_lib/misc.go +++ b/internal/imcontent/misc.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import ( "fmt" @@ -133,8 +133,8 @@ func formatCalendarContent(parsed map[string]interface{}, tag, extraAttrs string inner = append(inner, summary) } - start := formatTimestamp(startTime) - end := formatTimestamp(endTime) + start := FormatTimestamp(startTime) + end := FormatTimestamp(endTime) if start != "" && end != "" { inner = append(inner, start+" ~ "+end) } else if start != "" { @@ -213,13 +213,13 @@ func (todoConverter) Convert(ctx *ConvertContext) string { inner = append(inner, title) } if blocks, ok := summary["content"].([]interface{}); ok { - if text := extractPostBlocksText(blocks); text != "" { + if text := ExtractPostBlocksText(blocks); text != "" { inner = append(inner, text) } } } if dueTime, _ := parsed["due_time"].(string); dueTime != "" { - if formatted := formatTimestamp(dueTime); formatted != "" { + if formatted := FormatTimestamp(dueTime); formatted != "" { inner = append(inner, "Due: "+formatted) } } diff --git a/shortcuts/im/convert_lib/text.go b/internal/imcontent/text.go similarity index 96% rename from shortcuts/im/convert_lib/text.go rename to internal/imcontent/text.go index c047be7fd7..4e643ea0fc 100644 --- a/shortcuts/im/convert_lib/text.go +++ b/internal/imcontent/text.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import ( "fmt" @@ -97,6 +97,11 @@ func unwrapPostLocale(parsed map[string]interface{}) map[string]interface{} { return nil } +// UnwrapPostLocale returns the post body for the first supported locale shape. +func UnwrapPostLocale(parsed map[string]interface{}) map[string]interface{} { + return unwrapPostLocale(parsed) +} + // renderPostElem renders a single post (rich-text) element to its inline text // form: text/a/at carry their content through applyPostStyle for text.style // Markdown emphasis, emotion becomes :emoji_type:, md is passed through raw, diff --git a/shortcuts/im/convert_lib/text_test.go b/internal/imcontent/text_test.go similarity index 99% rename from shortcuts/im/convert_lib/text_test.go rename to internal/imcontent/text_test.go index 235b5857cd..4df877d8a1 100644 --- a/shortcuts/im/convert_lib/text_test.go +++ b/internal/imcontent/text_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2026 Lark Technologies Pte. Ltd. // SPDX-License-Identifier: MIT -package convertlib +package imcontent import "testing" diff --git a/internal/qualitygate/deptest/layering-edges.txt b/internal/qualitygate/deptest/layering-edges.txt index 4bf0bc39e0..32daa463a4 100644 --- a/internal/qualitygate/deptest/layering-edges.txt +++ b/internal/qualitygate/deptest/layering-edges.txt @@ -1,8 +1,4 @@ # from denied owner reason added_at -github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events root aggregates the existing im conversion dependency 2026-07-24 -github.com/larksuite/cli/events github.com/larksuite/cli/shortcuts/common arch-migration events root aggregates the existing im conversion dependency 2026-07-24 -github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/im/convert_lib arch-migration events->shortcuts inversion via convert_lib 2026-07-24 -github.com/larksuite/cli/events/im github.com/larksuite/cli/shortcuts/common arch-migration events->shortcuts inversion via convert_lib 2026-07-24 github.com/larksuite/cli/shortcuts/im github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 github.com/larksuite/cli/shortcuts/mail github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 github.com/larksuite/cli/shortcuts/minutes github.com/larksuite/cli/internal/auth arch-migration shortcut bypasses RuntimeContext gate 2026-07-24 diff --git a/shortcuts/im/convert_lib/content_convert.go b/shortcuts/im/convert_lib/content_convert.go index 1292b7d33c..1a1ee3fb5b 100644 --- a/shortcuts/im/convert_lib/content_convert.go +++ b/shortcuts/im/convert_lib/content_convert.go @@ -5,7 +5,6 @@ package convertlib import ( "encoding/json" - "fmt" "math" "net/url" "reflect" @@ -13,14 +12,10 @@ import ( "strings" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/imcontent" "github.com/larksuite/cli/shortcuts/common" ) -// ContentConverter defines the interface for converting a message type's raw content to human-readable text. -type ContentConverter interface { - Convert(ctx *ConvertContext) string -} - // ConvertContext holds all context needed for content conversion. type ConvertContext struct { RawContent string @@ -45,45 +40,12 @@ type ConvertContext struct { MergeForwardSubItems map[string][]map[string]interface{} } -// converters maps message types to their ContentConverter implementations. -var converters map[string]ContentConverter - -func init() { - converters = map[string]ContentConverter{ - "text": textConverter{}, - "post": postConverter{}, - "image": imageConverter{}, - "file": fileConverter{}, - "audio": audioMsgConverter{}, - "video": videoMsgConverter{}, - "media": videoMsgConverter{}, - "sticker": stickerConverter{}, - "interactive": interactiveConverter{}, - "share_chat": shareChatConverter{}, - "share_user": shareUserConverter{}, - "location": locationConverter{}, - "merge_forward": mergeForwardConverter{}, - "folder": folderConverter{}, - "share_calendar_event": calendarEventConverter{}, - "calendar": calendarInviteConverter{}, - "general_calendar": generalCalendarConverter{}, - "video_chat": videoChatConverter{}, - "system": systemConverter{}, - "todo": todoConverter{}, - "vote": voteConverter{}, - "hongbao": hongbaoConverter{}, - } -} - // ConvertBodyContent converts body.content (a raw JSON string) to human-readable text. func ConvertBodyContent(msgType string, ctx *ConvertContext) string { - if ctx.RawContent == "" { - return "" - } - if c, ok := converters[msgType]; ok { - return c.Convert(ctx) + if msgType == "merge_forward" { + return (mergeForwardConverter{}).Convert(ctx) } - return fmt.Sprintf("[%s]", msgType) + return imcontent.ConvertBodyContent(msgType, pureConvertContext(ctx)) } // FormatEventMessage converts an event-pushed message to a human-readable map. diff --git a/shortcuts/im/convert_lib/content_media_misc_test.go b/shortcuts/im/convert_lib/content_media_misc_test.go index b6b2be25db..0fe7efd290 100644 --- a/shortcuts/im/convert_lib/content_media_misc_test.go +++ b/shortcuts/im/convert_lib/content_media_misc_test.go @@ -464,26 +464,26 @@ func TestExtractMentionOpenIdAndTruncateContent(t *testing.T) { } func TestMediaConverters(t *testing.T) { - if got := (imageConverter{}).Convert(&ConvertContext{RawContent: `{"image_key":"img_1"}`}); got != "[Image: img_1]" { - t.Fatalf("imageConverter.Convert() = %q", got) + if got := convertPureForTest("image", `{"image_key":"img_1"}`); got != "[Image: img_1]" { + t.Fatalf("ConvertBodyContent(image) = %q", got) } - if got := (imageConverter{}).Convert(&ConvertContext{RawContent: `{invalid`}); got != "[Invalid image JSON]" { - t.Fatalf("imageConverter.Convert(invalid) = %q", got) + if got := convertPureForTest("image", `{invalid`); got != "[Invalid image JSON]" { + t.Fatalf("ConvertBodyContent(image invalid) = %q", got) } - if got := (fileConverter{}).Convert(&ConvertContext{RawContent: `{"file_key":"file_1","file_name":"demo.pdf"}`}); got != `` { - t.Fatalf("fileConverter.Convert() = %q", got) + if got := convertPureForTest("file", `{"file_key":"file_1","file_name":"demo.pdf"}`); got != `` { + t.Fatalf("ConvertBodyContent(file) = %q", got) } - if got := (fileConverter{}).Convert(&ConvertContext{RawContent: `{"file_key":"file_\"1","file_name":"demo\\\".pdf"}`}); got != `` { - t.Fatalf("fileConverter.Convert(escaped) = %q", got) + if got := convertPureForTest("file", `{"file_key":"file_\"1","file_name":"demo\\\".pdf"}`); got != `` { + t.Fatalf("ConvertBodyContent(file escaped) = %q", got) } - if got := (audioMsgConverter{}).Convert(&ConvertContext{RawContent: `{"duration":3500}`}); got != "[Voice: 4s]" { - t.Fatalf("audioMsgConverter.Convert() = %q", got) + if got := convertPureForTest("audio", `{"duration":3500}`); got != "[Voice: 4s]" { + t.Fatalf("ConvertBodyContent(audio) = %q", got) } - if got := (videoMsgConverter{}).Convert(&ConvertContext{RawContent: `{"file_key":"file_2","file_name":"clip.mp4","duration":5000,"image_key":"img_cover"}`}); got != `