From 0c25b26d33a7a3c22244e74f750356f71f7e47a8 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 10:59:29 +0800 Subject: [PATCH 01/21] fix: repair authsidecar_demo server-demo test compilation --- sidecar/server-demo/handler_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/sidecar/server-demo/handler_test.go b/sidecar/server-demo/handler_test.go index 2bc2a20f27..edd2446fef 100644 --- a/sidecar/server-demo/handler_test.go +++ b/sidecar/server-demo/handler_test.go @@ -18,6 +18,7 @@ import ( "testing" 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" @@ -585,11 +586,15 @@ func TestProxyHandler_StripsClientSuppliedAuthHeaders(t *testing.T) { } func TestBuildAllowedHosts(t *testing.T) { - feishu := struct{ Open, Accounts, MCP string }{ - "https://open.feishu.cn", "https://accounts.feishu.cn", "https://mcp.feishu.cn", - } - lark := struct{ Open, Accounts, MCP string }{ - "https://open.larksuite.com", "https://accounts.larksuite.com", "https://mcp.larksuite.com", + feishu := core.Endpoints{ + Open: "https://open.feishu.cn", + Accounts: "https://accounts.feishu.cn", + MCP: "https://mcp.feishu.cn", + } + lark := core.Endpoints{ + Open: "https://open.larksuite.com", + Accounts: "https://accounts.larksuite.com", + MCP: "https://mcp.larksuite.com", } hosts := buildAllowedHosts(feishu, lark) // feishu hosts From c2a1ef66ed3320a786e7ecd1795b3459e4170c49 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 11:12:58 +0800 Subject: [PATCH 02/21] test: add plugin_e2e L4 fork-build harness --- tests/plugin_e2e/harness.go | 175 +++++++++++++++++++++++++++++++++ tests/plugin_e2e/smoke_test.go | 68 +++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 tests/plugin_e2e/harness.go create mode 100644 tests/plugin_e2e/smoke_test.go diff --git a/tests/plugin_e2e/harness.go b/tests/plugin_e2e/harness.go new file mode 100644 index 0000000000..1fa44d8dbb --- /dev/null +++ b/tests/plugin_e2e/harness.go @@ -0,0 +1,175 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package plugin_e2e exercises the extension/platform plugin contract the way a +// real customer does: it builds a fork of lark-cli with a plugin blank-imported, +// then runs that fork as a subprocess and asserts the real stderr/stdout +// envelopes and exit codes. This is L4 coverage — the in-process unit and +// integration tests (extension/..., cmd/...) assert Go error values in the test +// process and structurally cannot observe envelope serialization, exit codes, or +// the blank-import -> init -> Register -> InstallAll assembly chain. +// +// Mechanism (the "customer build", mirrors xcaddy's build mode): +// 1. `git archive HEAD` a clean tree containing only committed files (so the +// fork embeds the tracked meta_data stub, reproducing the bare-module state). +// 2. Generate a customer module: go.mod (cli's requires + `replace` to the +// archived tree) + go.sum copy + main.go (blank-imports the plugin package) +// + plugin package (its init() calls platform.Register). +// 3. `go build` the fork (offline-capable via the warm module cache), then run +// it as a subprocess and assert. +package plugin_e2e + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// cleanTree is the git-archived, committed-only source tree of the repo under +// test, shared by every fork build. Populated by TestMain (smoke_test.go) — +// TestMain must live in a _test.go file to be recognized by `go test`, so the +// entry point sits there while the rest of the harness mechanism lives here. +var cleanTree string + +// baseDir holds the archive tree plus every generated customer module. +var baseDir string + +// repoRoot resolves the lark-cli module root from the test's working directory +// (which `go test` sets to the package dir, tests/plugin_e2e). +func repoRoot() (string, error) { + out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + +// gitArchive extracts HEAD's committed tree into dst. Only tracked files are +// included — gitignored build artifacts (e.g. the fetched meta_data.json) are +// absent, exactly as a module consumer would see them. +func gitArchive(root, dst string) error { + c := exec.Command("bash", "-c", "git archive HEAD | tar -x -C "+shellQuote(dst)) + c.Dir = root + return runCmd(c) +} + +func shellQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } + +// builtForks caches fork binaries by name so identical forks are built once. +var builtForks = map[string]string{} + +// buildFork generates a customer module whose plugin package body is pluginSrc, +// builds the fork, and returns the binary path. Forks are cached by name. +func buildFork(t *testing.T, name, pluginSrc string) string { + t.Helper() + if bin, ok := builtForks[name]; ok { + return bin + } + mod := filepath.Join(baseDir, "fork-"+name) + if err := os.MkdirAll(filepath.Join(mod, "plugin"), 0o755); err != nil { + t.Fatalf("mkdir customer module: %v", err) + } + + // go.mod: reuse cli's require graph, rename the module, replace cli with the + // local archived tree. This avoids `go mod tidy` (no network at test time). + rawMod, err := os.ReadFile(filepath.Join(cleanTree, "go.mod")) + if err != nil { + t.Fatalf("read archived go.mod: %v", err) + } + gomod := strings.Replace(string(rawMod), "module github.com/larksuite/cli", "module larkcustomer", 1) + gomod += "\nrequire github.com/larksuite/cli v0.0.0\n\nreplace github.com/larksuite/cli => " + cleanTree + "\n" + writeFile(t, filepath.Join(mod, "go.mod"), gomod) + + // go.sum: transitive dependency hashes are identical to cli's. + rawSum, err := os.ReadFile(filepath.Join(cleanTree, "go.sum")) + if err != nil { + t.Fatalf("read archived go.sum: %v", err) + } + writeFile(t, filepath.Join(mod, "go.sum"), string(rawSum)) + + writeFile(t, filepath.Join(mod, "main.go"), customerMain) + writeFile(t, filepath.Join(mod, "plugin", "plugin.go"), pluginSrc) + + bin := filepath.Join(mod, "fork-bin") + build := exec.Command("go", "build", "-o", bin, ".") + build.Dir = mod + // -mod=mod fixes require annotations copied from cli's go.mod; the default + // GOPROXY resolves any dep missing from the cache (goproxy in CI/dev). + build.Env = append(os.Environ(), "GOFLAGS=-mod=mod") + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("build fork %q failed: %v\n%s", name, err, out) + } + builtForks[name] = bin + return bin +} + +const customerMain = `// Code generated by plugin_e2e; DO NOT EDIT. +package main + +import ( + "os" + + "github.com/larksuite/cli/cmd" + _ "larkcustomer/plugin" // blank import triggers plugin init() -> platform.Register +) + +func main() { os.Exit(cmd.Execute()) } +` + +// result is a subprocess run outcome. +type result struct { + stdout string + stderr string + exit int +} + +// run executes the fork binary with args and captures stdout/stderr/exit. +// Notifier env vars are suppressed to keep envelopes clean. +func run(t *testing.T, bin string, args ...string) result { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + c := exec.CommandContext(ctx, bin, args...) + c.Env = append(os.Environ(), + "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", + "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", + ) + var stdout, stderr strings.Builder + c.Stdout = &stdout + c.Stderr = &stderr + err := c.Run() + exit := 0 + if err != nil { + if ee, ok := err.(*exec.ExitError); ok { + exit = ee.ExitCode() + } else { + t.Fatalf("run %v: %v", args, err) + } + } + return result{stdout: stdout.String(), stderr: stderr.String(), exit: exit} +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +func runCmd(c *exec.Cmd) error { + if out, err := c.CombinedOutput(); err != nil { + return &cmdError{err: err, out: out} + } + return nil +} + +type cmdError struct { + err error + out []byte +} + +func (e *cmdError) Error() string { return e.err.Error() + ": " + string(e.out) } diff --git a/tests/plugin_e2e/smoke_test.go b/tests/plugin_e2e/smoke_test.go new file mode 100644 index 0000000000..3c8a108e24 --- /dev/null +++ b/tests/plugin_e2e/smoke_test.go @@ -0,0 +1,68 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package plugin_e2e + +import ( + "os" + "path/filepath" + "testing" +) + +// TestMain archives HEAD's committed tree once for the whole package before +// any fork build runs. It lives here (not in harness.go) because `go test` +// only discovers TestMain in a _test.go file — a TestMain defined in a plain +// .go file is silently never invoked. +func TestMain(m *testing.M) { + root, err := repoRoot() + if err != nil { + panic("locate repo root: " + err.Error()) + } + baseDir, err = os.MkdirTemp("", "plugin-e2e-") + if err != nil { + panic("mkdtemp: " + err.Error()) + } + cleanTree = filepath.Join(baseDir, "larkcli-clean") + if err := os.MkdirAll(cleanTree, 0o755); err != nil { + panic("mkdir clean tree: " + err.Error()) + } + if err := gitArchive(root, cleanTree); err != nil { + panic("git archive: " + err.Error()) + } + code := m.Run() + _ = os.RemoveAll(baseDir) + os.Exit(code) +} + +// noopPlugin registers a plugin that installs nothing observable, proving +// the blank-import -> init -> Register -> InstallAll assembly chain links +// and the fork boots. +const noopPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("smoke", "0.0.1"). + Observer(platform.After, "noop", platform.All(), + func(_ context.Context, _ platform.Invocation) {}). + FailOpen(). + MustBuild()) +} +` + +func TestSmokeForkBoots(t *testing.T) { + bin := buildFork(t, "smoke", noopPlugin) + res := run(t, bin, "--help") + if res.exit != 0 { + t.Fatalf("--help exit=%d stderr=%s", res.exit, res.stderr) + } + if res.stdout == "" { + t.Fatalf("--help produced empty stdout") + } +} From 71c4c1fda363045f65856d693191112596db135f Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 11:27:59 +0800 Subject: [PATCH 03/21] test: cover restrict denial, allow-path and diagnostics in plugin_e2e --- tests/plugin_e2e/diagnostics_test.go | 39 +++++++++++++ tests/plugin_e2e/restrict_test.go | 84 ++++++++++++++++++++++++++++ tests/plugin_e2e/smoke_test.go | 1 + 3 files changed, 124 insertions(+) create mode 100644 tests/plugin_e2e/diagnostics_test.go create mode 100644 tests/plugin_e2e/restrict_test.go diff --git a/tests/plugin_e2e/diagnostics_test.go b/tests/plugin_e2e/diagnostics_test.go new file mode 100644 index 0000000000..a0cf25d579 --- /dev/null +++ b/tests/plugin_e2e/diagnostics_test.go @@ -0,0 +1,39 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package plugin_e2e + +import ( + "testing" + + "github.com/tidwall/gjson" +) + +// TestDiagnostics asserts the VERIFIED stdout shapes of the two policy/plugin +// diagnostic commands on a fork carrying the readonly Restrict rule: +// - `config policy show`: source_name == the plugin name that installed the +// active rule. +// - `config plugins show`: {"plugins":[{"name","version","capabilities",..., +// "hooks":{...}}],"total":N} with the readonly plugin present. +func TestDiagnostics(t *testing.T) { + bin := buildFork(t, "readonly", readonlyPlugin) + + pol := run(t, bin, "config", "policy", "show") + if pol.exit != 0 || !gjson.Valid(pol.stdout) { + t.Fatalf("policy show exit=%d stdout=%s stderr=%s", pol.exit, pol.stdout, pol.stderr) + } + if src := gjson.Get(pol.stdout, "source_name").String(); src != "readonly" { + t.Errorf("policy source_name=%q want readonly (stdout=%s)", src, pol.stdout) + } + + plug := run(t, bin, "config", "plugins", "show") + if plug.exit != 0 || !gjson.Valid(plug.stdout) { + t.Fatalf("plugins show exit=%d stdout=%s", plug.exit, plug.stdout) + } + if total := gjson.Get(plug.stdout, "total").Int(); total < 1 { + t.Errorf("plugins total=%d want >=1 (stdout=%s)", total, plug.stdout) + } + if name := gjson.Get(plug.stdout, "plugins.0.name").String(); name != "readonly" { + t.Errorf("plugins.0.name=%q want readonly", name) + } +} diff --git a/tests/plugin_e2e/restrict_test.go b/tests/plugin_e2e/restrict_test.go new file mode 100644 index 0000000000..a70b7ec724 --- /dev/null +++ b/tests/plugin_e2e/restrict_test.go @@ -0,0 +1,84 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package plugin_e2e + +import ( + "strings" + "testing" + + "github.com/tidwall/gjson" +) + +// readonlyPlugin registers a Restrict rule that only allows read-risk +// commands under the docs/** and im/** domains. It mirrors the official +// example readonly-policy configuration. +const readonlyPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register( + platform.NewPlugin("readonly", "0.1.0"). + Restrict(&platform.Rule{ + Name: "agent-readonly", + Allow: []string{"docs/**", "im/**"}, + MaxRisk: platform.RiskRead, + }). + MustBuild()) +} +` + +// TestReadonlyDenial asserts the VERIFIED denial envelope shape: stderr is +// valid JSON, error.type=="validation", error.subtype=="failed_precondition", +// error.hint contains the literal "reason_code " substring, and the +// process exits 2. reason_code lives only in the hint string, not a +// structured field. +func TestReadonlyDenial(t *testing.T) { + bin := buildFork(t, "readonly", readonlyPlugin) + cases := []struct { + name string + args []string + reasonCode string + }{ + {"write in allowed domain", []string{"docs", "+update", "--doc-token", "x", "--content", "y"}, "write_not_allowed"}, + {"leaf out of allow list", []string{"schema"}, "domain_not_allowed"}, + {"parent group all children denied", []string{"sheets"}, "mixed_children_policy"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + res := run(t, bin, tc.args...) + if res.exit != 2 { + t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, "reason_code "+tc.reasonCode) { + t.Errorf("hint=%q want to contain reason_code %s", hint, tc.reasonCode) + } + }) + } +} + +// TestReadonlyAllows asserts the allow-path: a read command inside an +// allowed domain must NOT be denied by the policy gate. It may still fail +// downstream (e.g. api/auth error), but that failure must not carry the +// denial envelope shape and must not exit 2. +func TestReadonlyAllows(t *testing.T) { + bin := buildFork(t, "readonly", readonlyPlugin) + res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") + if res.exit == 2 { + t.Fatalf("read command was denied (exit=2); stderr=%s", res.stderr) + } + if gjson.Get(res.stderr, "error.subtype").String() == "failed_precondition" { + t.Errorf("read command produced a denial envelope; stderr=%s", res.stderr) + } +} diff --git a/tests/plugin_e2e/smoke_test.go b/tests/plugin_e2e/smoke_test.go index 3c8a108e24..01c9c6f64d 100644 --- a/tests/plugin_e2e/smoke_test.go +++ b/tests/plugin_e2e/smoke_test.go @@ -13,6 +13,7 @@ import ( // any fork build runs. It lives here (not in harness.go) because `go test` // only discovers TestMain in a _test.go file — a TestMain defined in a plain // .go file is silently never invoked. +// NOTE: exactly one TestMain is allowed per package — do not add another in other _test.go files here. func TestMain(m *testing.M) { root, err := repoRoot() if err != nil { From 9ef612bc4a2a909c44129686efc5cc18014e7a97 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 13:04:03 +0800 Subject: [PATCH 04/21] test: cover identity/denylist/multi-rule denial, observe and wrap in plugin_e2e --- tests/plugin_e2e/observe_wrap_test.go | 297 ++++++++++++++++++++++++++ tests/plugin_e2e/restrict_test.go | 127 ++++++++++- 2 files changed, 423 insertions(+), 1 deletion(-) create mode 100644 tests/plugin_e2e/observe_wrap_test.go diff --git a/tests/plugin_e2e/observe_wrap_test.go b/tests/plugin_e2e/observe_wrap_test.go new file mode 100644 index 0000000000..46386fb8b8 --- /dev/null +++ b/tests/plugin_e2e/observe_wrap_test.go @@ -0,0 +1,297 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package plugin_e2e + +import ( + "strings" + "testing" + + "github.com/tidwall/gjson" +) + +// auditPlugin registers a single After observer matching every command that +// logs "[audit] " to stderr. Mirrors the shipped +// extension/platform/examples/audit-observer example. +const auditPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + "fmt" + "os" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("audit", "0.1.0"). + Observer(platform.After, "log", platform.All(), + func(_ context.Context, inv platform.Invocation) { + fmt.Fprintf(os.Stderr, "[audit] %s\n", inv.Cmd().Path()) + }). + FailOpen(). + MustBuild()) +} +` + +// TestObservePin pins the audit observer's stderr line format. Observed +// real output (docs +fetch --doc nonexistent, a real read-risk command that +// fails downstream with an API error unrelated to the plugin): +// +// exit=1 +// stderr=[audit] docs/+fetch +// {"ok":false,"identity":"user","error":{"type":"api","subtype":"unknown",...}} +// +// The observer line always leads, on its own line, before whatever the +// command itself writes to stderr. +func TestObservePin(t *testing.T) { + bin := buildFork(t, "audit", auditPlugin) + res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") + if !strings.Contains(res.stderr, "[audit] docs/+fetch\n") { + t.Fatalf("stderr missing audit line; stderr=%s", res.stderr) + } +} + +// auditRestrictPlugin combines an After observer with a Restrict rule in one +// plugin, so a denied command's stderr carries both the observer's +// side-effect and the denial envelope: the framework's contract is that +// After observers fire even for denied commands (see +// extension/platform/invocation.go's DeniedByPolicy doc). +const auditRestrictPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + "fmt" + "os" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("audit-restrict", "0.1.0"). + Observer(platform.After, "log", platform.All(), + func(_ context.Context, inv platform.Invocation) { + fmt.Fprintf(os.Stderr, "[audit] %s\n", inv.Cmd().Path()) + }). + Restrict(&platform.Rule{ + Name: "agent-readonly", + Allow: []string{"docs/**", "im/**"}, + MaxRisk: platform.RiskRead, + }). + MustBuild()) +} +` + +// TestObserveOnDeniedPin pins the audit-contract case: a denied command's +// stderr carries BOTH the observer's audit line AND the denial envelope, +// concatenated in a single stream, audit line first. Observed real output +// (docs +update --doc-token x --content y, denied write_not_allowed): +// +// exit=2 +// stderr=[audit] docs/+update +// {"ok":false,"error":{"type":"validation","subtype":"failed_precondition",...}} +// +// The leading "[audit] ..." line means gjson.Valid on the raw stderr is +// false; the JSON envelope must be sliced out from the first '{' before +// parsing it as JSON. +func TestObserveOnDeniedPin(t *testing.T) { + bin := buildFork(t, "audit-restrict", auditRestrictPlugin) + res := run(t, bin, "docs", "+update", "--doc-token", "x", "--content", "y") + if res.exit != 2 { + t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !strings.Contains(res.stderr, "[audit] docs/+update\n") { + t.Fatalf("stderr missing audit line on a denied command; stderr=%s", res.stderr) + } + i := strings.Index(res.stderr, "{") + if i < 0 { + t.Fatalf("stderr has no JSON envelope after the audit line; stderr=%s", res.stderr) + } + envelope := res.stderr[i:] + if !gjson.Valid(envelope) { + t.Fatalf("sliced envelope not JSON: %s", envelope) + } + if got := gjson.Get(envelope, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(envelope, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if hint := gjson.Get(envelope, "error.hint").String(); !strings.Contains(hint, "reason_code write_not_allowed") { + t.Errorf("hint=%q want to contain reason_code write_not_allowed", hint) + } +} + +// observerPanicPlugin's After observer panics unconditionally. runObserverSafe +// (internal/hook/install.go) must isolate the panic so command dispatch +// still completes normally. +const observerPanicPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("observer-panic", "0.1.0"). + Observer(platform.After, "log", platform.All(), + func(_ context.Context, _ platform.Invocation) { + panic("boom") + }). + FailOpen(). + MustBuild()) +} +` + +// TestObserverPanicIsolationPin pins panic isolation: an After observer that +// always panics must not affect the command's own outcome. Observed real +// output for `schema` (a local, network-free, read-risk command) under the +// panicking-observer fork vs. the noop-observer baseline fork: +// +// panicking: exit=0 stderr=warning: hook "observer-panic.log" panicked: boom +// baseline: exit=0 stderr=(empty) +// +// Both exit 0 identically; the panic is fully swallowed by +// runObserverSafe (internal/hook/install.go), surfacing only as a stderr +// warning line, never as a non-zero exit or crash. +func TestObserverPanicIsolationPin(t *testing.T) { + bin := buildFork(t, "observer-panic", observerPanicPlugin) + res := run(t, bin, "schema") + + baselineBin := buildFork(t, "smoke", noopPlugin) + baseline := run(t, baselineBin, "schema") + + if res.exit != baseline.exit { + t.Fatalf("panicking-observer exit=%d differs from baseline exit=%d; stderr=%s", res.exit, baseline.exit, res.stderr) + } + if !strings.Contains(res.stderr, `warning: hook "observer-panic.log" panicked: boom`) { + t.Errorf("stderr missing panic-isolation warning; stderr=%s", res.stderr) + } +} + +// wrapAbortPlugin's Wrapper short-circuits every command with an AbortError +// instead of calling next. +const wrapAbortPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("wrap-abort", "0.1.0"). + Wrap("guard", platform.All(), func(next platform.Handler) platform.Handler { + return func(ctx context.Context, inv platform.Invocation) error { + return &platform.AbortError{ + HookName: "guard", + Reason: "blocked for test", + } + } + }). + FailOpen(). + MustBuild()) +} +` + +// TestWrapAbortPin pins the wrap-abort envelope shape. An *AbortError +// returned by a Wrapper is converted by wrapAbortError +// (internal/hook/install.go) into the SAME envelope shape as a Restrict +// denial -- error.type=="validation", error.subtype=="failed_precondition" +// -- NOT a distinct "hook" error type. Observed real output (docs +fetch +// --doc nonexistent, wrapper aborts unconditionally before calling next): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"hook \"wrap-abort.guard\" aborted: blocked for test", +// "hint":"plugin hook \"wrap-abort.guard\" aborted this command; adjust the +// request to satisfy the hook's policy, or remove the plugin"}} +// +// HookName is namespaced to "." ("wrap-abort.guard") +// regardless of the HookName the plugin set on the AbortError itself +// (namespacedWrap overwrites it) -- see internal/hook/install.go. +func TestWrapAbortPin(t *testing.T) { + bin := buildFork(t, "wrap-abort", wrapAbortPlugin) + res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") + if res.exit != 2 { + t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if msg := gjson.Get(res.stderr, "error.message").String(); !strings.Contains(msg, `hook "wrap-abort.guard" aborted: blocked for test`) { + t.Errorf("error.message=%q want to contain the namespaced hook name and Reason", msg) + } + if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, `plugin hook "wrap-abort.guard" aborted this command`) { + t.Errorf("error.hint=%q want to contain the abort hint", hint) + } +} + +// wrapPanicPlugin's Wrapper factory panics on every invocation (the factory +// closure itself, not the returned Handler). +const wrapPanicPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("wrap-panic", "0.1.0"). + Wrap("guard", platform.All(), func(next platform.Handler) platform.Handler { + panic("wrap boom") + }). + FailOpen(). + MustBuild()) +} +` + +// TestWrapPanicPin pins the wrap-panic envelope shape: a panicking Wrapper +// factory does not crash the process. recoverWrap (internal/hook/install.go) +// converts the panic into the same validation/failed_precondition shape as +// wrap-abort, with a distinct message/hint pair. Observed real output (docs +// +fetch --doc nonexistent, wrapper factory panics unconditionally): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"hook \"wrap-panic.guard\" panicked: wrap boom", +// "hint":"plugin hook \"wrap-panic.guard\" crashed while handling this +// command; report the panic to the plugin author or remove the plugin"}} +func TestWrapPanicPin(t *testing.T) { + bin := buildFork(t, "wrap-panic", wrapPanicPlugin) + res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") + if res.exit != 2 { + t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if msg := gjson.Get(res.stderr, "error.message").String(); !strings.Contains(msg, `hook "wrap-panic.guard" panicked: wrap boom`) { + t.Errorf("error.message=%q want to contain the namespaced hook name and panic value", msg) + } + if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, `plugin hook "wrap-panic.guard" crashed while handling this command`) { + t.Errorf("error.hint=%q want to contain the panic hint", hint) + } +} diff --git a/tests/plugin_e2e/restrict_test.go b/tests/plugin_e2e/restrict_test.go index a70b7ec724..d1897d54b4 100644 --- a/tests/plugin_e2e/restrict_test.go +++ b/tests/plugin_e2e/restrict_test.go @@ -78,7 +78,132 @@ func TestReadonlyAllows(t *testing.T) { if res.exit == 2 { t.Fatalf("read command was denied (exit=2); stderr=%s", res.stderr) } - if gjson.Get(res.stderr, "error.subtype").String() == "failed_precondition" { + if gjson.Valid(res.stderr) && gjson.Get(res.stderr, "error.subtype").String() == "failed_precondition" { t.Errorf("read command produced a denial envelope; stderr=%s", res.stderr) } } + +// identityPlugin registers a Restrict rule scoped to bot identities only. +// im +messages-search declares AuthTypes:["user"] (see +// shortcuts/im/im_messages_search.go), so it has no intersection with the +// rule's bot-only whitelist regardless of which --as value the caller +// passes: platform.Rule.Identities is checked against the command's own +// static supported-identities annotation, not the runtime --as flag. +const identityPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register( + platform.NewPlugin("identity-restrict", "0.1.0"). + Restrict(&platform.Rule{ + Name: "bot-only", + Allow: []string{"im/**"}, + MaxRisk: platform.RiskRead, + Identities: []platform.Identity{platform.IdentityBot}, + }). + MustBuild()) +} +` + +// denylistPlugin registers a Restrict rule that allows the docs/** domain +// but explicitly denies docs/+search (a real read-risk leaf, see +// shortcuts/doc/docs_search.go). Deny has priority over Allow, so the +// command is rejected before MaxRisk is even consulted. +const denylistPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register( + platform.NewPlugin("denylist-restrict", "0.1.0"). + Restrict(&platform.Rule{ + Name: "deny-search", + Allow: []string{"docs/**"}, + Deny: []string{"docs/+search"}, + MaxRisk: platform.RiskRead, + }). + MustBuild()) +} +` + +// multiRulePlugin registers two scope-exclusive Restrict rules (im-only, +// docs-only). A command outside both domains (e.g. the top-level "schema" +// command, itself read-risk and already proven to hit domain_not_allowed +// under a single Allow:["docs/**","im/**"] rule in TestReadonlyDenial) is +// rejected by both rules, so cmdpolicy's OR-engine collapses the two +// per-rule denials into the aggregate reason_code "no_matching_rule". +const multiRulePlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register( + platform.NewPlugin("multi-rule-restrict", "0.1.0"). + Restrict(&platform.Rule{ + Name: "im-only", + Allow: []string{"im/**"}, + MaxRisk: platform.RiskRead, + }). + Restrict(&platform.Rule{ + Name: "docs-only", + Allow: []string{"docs/**"}, + MaxRisk: platform.RiskRead, + }). + MustBuild()) +} +` + +// assertDenialEnvelope asserts the VERIFIED denial envelope shape shared by +// every reason_code in this file: exit 2, valid JSON on stderr, +// error.type=="validation", error.subtype=="failed_precondition", and +// error.hint containing "reason_code ". +func assertDenialEnvelope(t *testing.T, res result, wantReasonCode string) { + t.Helper() + if res.exit != 2 { + t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, "reason_code "+wantReasonCode) { + t.Errorf("hint=%q want to contain reason_code %s", hint, wantReasonCode) + } +} + +// TestIdentityMismatchDenial pins reason_code=identity_mismatch: a bot-only +// rule rejects a command whose declared AuthTypes don't include "bot". +func TestIdentityMismatchDenial(t *testing.T) { + bin := buildFork(t, "identity", identityPlugin) + res := run(t, bin, "im", "+messages-search", "--as", "user") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertDenialEnvelope(t, res, "identity_mismatch") +} + +// TestDenylistDenial pins reason_code=command_denylisted: a Deny glob hit +// rejects the command even though it also matches Allow. +func TestDenylistDenial(t *testing.T) { + bin := buildFork(t, "denylist", denylistPlugin) + res := run(t, bin, "docs", "+search") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertDenialEnvelope(t, res, "command_denylisted") +} + +// TestMultiRuleDenial pins reason_code=no_matching_rule: a command rejected +// by every rule in a multi-Restrict() plugin gets the aggregate reason_code, +// not either rule's own per-rule reason_code. +func TestMultiRuleDenial(t *testing.T) { + bin := buildFork(t, "multirule", multiRulePlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertDenialEnvelope(t, res, "no_matching_rule") +} From 9e5bf906fdf552316c8b1d954056e8e21d0ac298 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 13:20:49 +0800 Subject: [PATCH 05/21] test: cover install-time reason_codes on broken plugin forks --- tests/plugin_e2e/install_test.go | 367 +++++++++++++++++++++++++++++++ 1 file changed, 367 insertions(+) create mode 100644 tests/plugin_e2e/install_test.go diff --git a/tests/plugin_e2e/install_test.go b/tests/plugin_e2e/install_test.go new file mode 100644 index 0000000000..2ef5fdba3b --- /dev/null +++ b/tests/plugin_e2e/install_test.go @@ -0,0 +1,367 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package plugin_e2e + +import ( + "strings" + "testing" + + "github.com/tidwall/gjson" +) + +// assertInstallEnvelope asserts the VERIFIED install-time failure envelope +// shape shared by every reason_code in this file: exit 2, valid JSON on +// stderr, error.type=="validation", error.subtype=="failed_precondition", +// and error.hint containing "reason_code ". This mirrors +// assertDenialEnvelope in restrict_test.go -- install-time failures render +// through the SAME cmd/platform_guards.go WithHint(...) family as policy +// denials, embedding reason_code in the hint STRING, not a structured +// error.detail.reason_code field (contradicting the candidate shape +// referenced from internal/platform/error.go:34's comment). +func assertInstallEnvelope(t *testing.T, res result, wantReasonCode string) { + t.Helper() + if res.exit != 2 { + t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, "reason_code "+wantReasonCode) { + t.Errorf("hint=%q want to contain reason_code %s", hint, wantReasonCode) + } +} + +// multipleRestrictPlugin registers TWO distinct plugins that each call +// Restrict() with an independently valid Rule. cmdpolicy.Resolve rejects +// more than one distinct Restrict-owner regardless of each rule's own +// validity (internal/cmdpolicy/resolver.go's distinctOwners check runs +// before ValidateRule). +const multipleRestrictPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register( + platform.NewPlugin("restrict-a", "0.1.0"). + Restrict(&platform.Rule{ + Name: "a-rule", + Allow: []string{"docs/**"}, + MaxRisk: platform.RiskRead, + }). + MustBuild()) + platform.Register( + platform.NewPlugin("restrict-b", "0.1.0"). + Restrict(&platform.Rule{ + Name: "b-rule", + Allow: []string{"im/**"}, + MaxRisk: platform.RiskRead, + }). + MustBuild()) +} +` + +// TestInstallMultipleRestrictPluginsPin pins reason_code=multiple_restrict_plugins. +// Observed real output (any command, e.g. "schema" -- the fatal guard walks +// every RunE in the tree so it fires regardless of which command runs): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"multiple plugins called Restrict; only one plugin may own the +// policy: [restrict-a restrict-b]", +// "hint":"plugin policy configuration is broken (reason_code +// multiple_restrict_plugins); fix the plugin's Restrict rule or remove the +// conflicting plugin"}} +func TestInstallMultipleRestrictPluginsPin(t *testing.T) { + bin := buildFork(t, "multiple-restrict", multipleRestrictPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "multiple_restrict_plugins") +} + +// invalidRulePlugin registers a single plugin whose Restrict Rule carries a +// syntactically-invalid MaxRisk value. Neither the Builder nor the staging +// Registrar validate Rule *contents* (only nilness) -- semantic validation +// happens later, in cmdpolicy.ValidateRule, called from +// cmd/platform_bootstrap.go's applyUserPolicyPruning -> cmdpolicy.Resolve. +const invalidRulePlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register( + platform.NewPlugin("invalid-rule", "0.1.0"). + Restrict(&platform.Rule{ + Name: "bad-risk", + Allow: []string{"docs/**"}, + MaxRisk: platform.Risk("bogus"), + }). + MustBuild()) +} +` + +// TestInstallInvalidRulePin pins reason_code=invalid_rule. Observed real output +// (schema): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"plugin \"invalid-rule\" rule invalid: invalid max_risk \"bogus\": +// must be one of read|write|high-risk-write", +// "hint":"plugin policy configuration is broken (reason_code invalid_rule); +// fix the plugin's Restrict rule or remove the conflicting plugin"}} +func TestInstallInvalidRulePin(t *testing.T) { + bin := buildFork(t, "invalid-rule", invalidRulePlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "invalid_rule") +} + +// installFailedPlugin is a hand-written bare platform.Plugin (not +// Builder-based -- Install returning a plain error is not expressible +// through the Builder's fluent API) whose Install always returns an error. +// FailurePolicy=FailClosed makes the host abort rather than warn+skip. +const installFailedPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "errors" + + "github.com/larksuite/cli/extension/platform" +) + +type installFailed struct{} + +func (installFailed) Name() string { return "install-failed" } +func (installFailed) Version() string { return "0.1.0" } +func (installFailed) Capabilities() platform.Capabilities { + return platform.Capabilities{FailurePolicy: platform.FailClosed} +} +func (installFailed) Install(r platform.Registrar) error { + return errors.New("deliberate install failure") +} + +func init() { platform.Register(installFailed{}) } +` + +// TestInstallFailedPin pins reason_code=install_failed. Observed real output +// (schema): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"plugin \"install-failed\" (install_failed): Install returned +// error: deliberate install failure", +// "hint":"plugin \"install-failed\" failed to install (reason_code +// install_failed); fix or remove the plugin before running commands"}} +func TestInstallFailedPin(t *testing.T) { + bin := buildFork(t, "install-failed", installFailedPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "install_failed") +} + +// installPanicPlugin is a hand-written bare Plugin whose Install panics. +// safeCallInstall (internal/platform/host.go) recovers and converts the +// panic into a typed install_panic error rather than crashing the binary. +const installPanicPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +type installPanic struct{} + +func (installPanic) Name() string { return "install-panic" } +func (installPanic) Version() string { return "0.1.0" } +func (installPanic) Capabilities() platform.Capabilities { + return platform.Capabilities{FailurePolicy: platform.FailClosed} +} +func (installPanic) Install(r platform.Registrar) error { + panic("deliberate install panic") +} + +func init() { platform.Register(installPanic{}) } +` + +// TestInstallPanicPin pins reason_code=install_panic. Observed real output +// (schema): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"plugin \"install-panic\" (install_panic): Install panicked: +// deliberate install panic", +// "hint":"plugin \"install-panic\" failed to install (reason_code +// install_panic); fix or remove the plugin before running commands"}} +func TestInstallPanicPin(t *testing.T) { + bin := buildFork(t, "install-panic", installPanicPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "install_panic") +} + +// pluginNamePanicPlugin is a hand-written bare Plugin whose Name() panics. +// InstallAll's outer loop calls safeCallName BEFORE it ever reads +// Capabilities(), so this aborts unconditionally regardless of what +// Capabilities() would have declared (host.go's isUntrustedConfigError +// list) -- Capabilities() here is a throwaway zero value, never invoked. +const pluginNamePanicPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +type pluginNamePanic struct{} + +func (pluginNamePanic) Name() string { panic("deliberate name panic") } +func (pluginNamePanic) Version() string { return "0.1.0" } +func (pluginNamePanic) Capabilities() platform.Capabilities { + return platform.Capabilities{} +} +func (pluginNamePanic) Install(r platform.Registrar) error { return nil } + +func init() { platform.Register(pluginNamePanic{}) } +` + +// TestInstallPluginNamePanicPin pins reason_code=plugin_name_panic. Observed real +// output (schema): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"plugin \"\" (plugin_name_panic): Plugin.Name() +// panicked: deliberate name panic", +// "hint":"plugin \"\" failed to install (reason_code +// plugin_name_panic); fix or remove the plugin before running commands"}} +func TestInstallPluginNamePanicPin(t *testing.T) { + bin := buildFork(t, "plugin-name-panic", pluginNamePanicPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "plugin_name_panic") +} + +// capabilitiesPanicPlugin is a hand-written bare Plugin whose Capabilities() +// panics. readFailurePolicy (internal/platform/host.go) re-invokes +// Capabilities() to decide FailOpen vs FailClosed, panics again, and its +// recover leaves the pre-set FailClosed default in place -- so this aborts +// unconditionally too, without the plugin ever declaring a real policy. +const capabilitiesPanicPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +type capabilitiesPanic struct{} + +func (capabilitiesPanic) Name() string { return "capabilities-panic" } +func (capabilitiesPanic) Version() string { return "0.1.0" } +func (capabilitiesPanic) Capabilities() platform.Capabilities { + panic("deliberate capabilities panic") +} +func (capabilitiesPanic) Install(r platform.Registrar) error { return nil } + +func init() { platform.Register(capabilitiesPanic{}) } +` + +// TestInstallCapabilitiesPanicPin pins reason_code=capabilities_panic. Observed +// real output (schema): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"plugin \"capabilities-panic\" (capabilities_panic): +// Plugin.Capabilities() panicked: deliberate capabilities panic", +// "hint":"plugin \"capabilities-panic\" failed to install (reason_code +// capabilities_panic); fix or remove the plugin before running commands"}} +func TestInstallCapabilitiesPanicPin(t *testing.T) { + bin := buildFork(t, "capabilities-panic", capabilitiesPanicPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "capabilities_panic") +} + +// restrictsMismatchPlugin is a hand-written bare Plugin that declares +// Capabilities.Restricts=true (paired with the required FailClosed) but +// whose Install never calls r.Restrict. stagingRegistrar.validateSelf +// (internal/platform/staging.go) checks this exact declared-vs-actual +// consistency after Install returns. +const restrictsMismatchPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +type restrictsMismatch struct{} + +func (restrictsMismatch) Name() string { return "restricts-mismatch" } +func (restrictsMismatch) Version() string { return "0.1.0" } +func (restrictsMismatch) Capabilities() platform.Capabilities { + return platform.Capabilities{Restricts: true, FailurePolicy: platform.FailClosed} +} +func (restrictsMismatch) Install(r platform.Registrar) error { return nil } + +func init() { platform.Register(restrictsMismatch{}) } +` + +// TestInstallRestrictsMismatchPin pins reason_code=restricts_mismatch. +// Observed real output (schema): +// +// exit=2 +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"plugin \"restricts-mismatch\" (restricts_mismatch): +// Capabilities.Restricts=true but Install did not call r.Restrict", +// "hint":"plugin \"restricts-mismatch\" failed to install (reason_code +// restricts_mismatch); fix or remove the plugin before running commands"}} +func TestInstallRestrictsMismatchPin(t *testing.T) { + bin := buildFork(t, "restricts-mismatch", restrictsMismatchPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + assertInstallEnvelope(t, res, "restricts_mismatch") +} + +// mustBuildPanicPlugin calls MustBuild() on a Builder with an invalid plugin +// name ("BadName!!" fails ^[a-z0-9][a-z0-9-]*$). This panics from +// plugin.init(), which runs from the blank-import BEFORE main() has a +// chance to install any recover-and-envelope guard -- so, unlike every +// other case here, this crashes the process outright: no JSON envelope, +// non-zero exit, a raw Go panic trace on stderr. +const mustBuildPanicPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import "github.com/larksuite/cli/extension/platform" + +func init() { + platform.Register(platform.NewPlugin("BadName!!", "0.1.0").MustBuild()) +} +` + +// TestInstallMustBuildInitPanicCrashesBinary pins the MustBuild init-panic crash +// shape. This is NOT the plugin_install envelope -- it is a bare Go panic +// crash, because it happens in init(), before main()'s recover guard +// exists. Observed real output (schema): +// +// exit=2 +// stderr=panic: plugin "BadName!!": invalid plugin name "BadName!!": must +// match ^[a-z0-9][a-z0-9-]*$ +// +// goroutine 1 [running]: +// larkcustomer/plugin.init.0(...) +// .../plugin/plugin.go:7 +// ... +func TestInstallMustBuildInitPanicCrashesBinary(t *testing.T) { + bin := buildFork(t, "mustbuild-panic", mustBuildPanicPlugin) + res := run(t, bin, "schema") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + if res.exit == 0 { + t.Fatalf("expected non-zero exit on init panic; exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if gjson.Valid(res.stderr) { + t.Fatalf("expected a raw panic trace, not a JSON envelope; stderr=%s", res.stderr) + } + if !strings.Contains(res.stderr, "panic:") { + t.Fatalf("stderr missing Go panic trace; stderr=%s", res.stderr) + } + if !strings.Contains(res.stderr, `invalid plugin name "BadName!!"`) { + t.Errorf("stderr missing the Builder's invalid-name message; stderr=%s", res.stderr) + } +} From 10e860ffd2663e567481bb7915e291f12f615a5f Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 14:19:16 +0800 Subject: [PATCH 06/21] test: cover stub-metadata degrade and offline subsystem forks --- tests/plugin_e2e/degrade_subsystem_test.go | 278 +++++++++++++++++++++ 1 file changed, 278 insertions(+) create mode 100644 tests/plugin_e2e/degrade_subsystem_test.go diff --git a/tests/plugin_e2e/degrade_subsystem_test.go b/tests/plugin_e2e/degrade_subsystem_test.go new file mode 100644 index 0000000000..772f097081 --- /dev/null +++ b/tests/plugin_e2e/degrade_subsystem_test.go @@ -0,0 +1,278 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package plugin_e2e + +import ( + "context" + "os" + "os/exec" + "strings" + "testing" + "time" + + "github.com/tidwall/gjson" +) + +// runIsolated runs bin like run() (harness.go), but with a per-call +// LARKSUITE_CLI_CONFIG_DIR (t.TempDir()) and LARKSUITE_CLI_REMOTE_META=off. +// This is needed ONLY by TestDegradeStubMetadataSchema below: without it, the +// fork inherits this developer machine's real ~/.lark-cli cache (a prior +// `lark-cli` invocation on this box already populated remote_meta.json) and/or +// makes a live network fetch, either of which would populate the runtime +// catalog with real data and hide the #1764 stub-metadata degrade path this +// test exists to pin. harness.go's run() has no env-override parameter and +// must not be modified to add one (out of scope for this task), so this is a +// small local variant of the same subprocess-capture logic. +func runIsolated(t *testing.T, bin string, args ...string) result { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + c := exec.CommandContext(ctx, bin, args...) + c.Env = append(os.Environ(), + "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", + "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", + "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), + "LARKSUITE_CLI_REMOTE_META=off", + ) + var stdout, stderr strings.Builder + c.Stdout = &stdout + c.Stderr = &stderr + err := c.Run() + exit := 0 + if err != nil { + if ee, ok := err.(*exec.ExitError); ok { + exit = ee.ExitCode() + } else { + t.Fatalf("run %v: %v", args, err) + } + } + return result{stdout: stdout.String(), stderr: stderr.String(), exit: exit} +} + +// plainPlugin registers a minimal observer-only plugin with NO Restrict rule +// -- unlike readonly_test.go's plugins, it cannot deny "schema" as +// out-of-domain, so any failure the command produces below is the command's +// own behavior against the empty stub catalog, not a policy denial. +const plainPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + + "github.com/larksuite/cli/extension/platform" +) + +func init() { + platform.Register( + platform.NewPlugin("plain", "0.1.0"). + Observer(platform.After, "noop", platform.All(), + func(_ context.Context, _ platform.Invocation) {}). + FailOpen(). + MustBuild()) +} +` + +// TestDegradeStubMetadataSchema pins the #1764 stub-metadata degrade path. +// The clean tree embeds only the empty meta_data_default.json stub +// (internal/registry/catalog.go's SchemaCatalog falls through to +// RuntimeCatalog when EmbeddedServicesTyped() is empty), and runIsolated +// additionally disables the remote overlay fetch and points the cache dir at +// an empty tmp dir, so cmd/schema/schema.go's runSchema sees +// catalog.Services() == 0 unconditionally -- the exact "offline with a cold +// cache, remote meta off" branch documented at cmd/schema/schema.go:96-101. +// +// Observed real output for both `schema` and `schema im.messages.reply` +// (identical -- runSchema checks catalog.Services()==0 before parsing args): +// +// exit=2 +// stdout=(empty) +// stderr={"ok":false,"error":{"type":"validation","subtype":"failed_precondition", +// "message":"No API metadata available", +// "hint":"this binary has no embedded API metadata; run any command with +// network access to the open platform once so metadata can be fetched and +// cached"}} +// +// This is the PINNED "graceful degrade" criterion: a structured JSON envelope +// (gjson.Valid, no "panic:" substring) carrying a validation/failed_precondition +// error with an actionable hint, NOT the raw Go panic crash that +// install_test.go's TestInstallMustBuildInitPanicCrashesBinary pins for a +// genuinely broken plugin, and NOT an "Unknown"-shaped internal error. +// Note: exit==2 alone does not prove "not a crash" -- a genuine Go panic also +// exits 2. The two real discriminators against a crash are the absence of a +// "panic:" substring in stderr and stderr being valid JSON (gjson.Valid); both +// are asserted below. +func TestDegradeStubMetadataSchema(t *testing.T) { + bin := buildFork(t, "plain", plainPlugin) + cases := []struct { + name string + args []string + }{ + {"schema root", []string{"schema"}}, + {"schema with path", []string{"schema", "im.messages.reply"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + res := runIsolated(t, bin, tc.args...) + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + if res.exit != 2 { + t.Fatalf("exit=%d want 2 (graceful validation exit); stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if strings.Contains(res.stderr, "panic:") { + t.Fatalf("stderr contains a raw Go panic trace, not a graceful degrade; stderr=%s", res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not a structured JSON envelope: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { + t.Errorf("error.type=%q want validation", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { + t.Errorf("error.subtype=%q want failed_precondition", got) + } + if msg := gjson.Get(res.stderr, "error.message").String(); msg != "No API metadata available" { + t.Errorf("error.message=%q want %q", msg, "No API metadata available") + } + if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, "no embedded API metadata") { + t.Errorf("error.hint=%q want to contain %q", hint, "no embedded API metadata") + } + }) + } +} + +// transportAbortPlugin registers a transport.Provider whose Interceptor +// implements AbortableInterceptor and unconditionally rejects every request +// from PreRoundTripE, before the built-in RoundTripper chain (and therefore +// before any real network I/O) ever runs -- see extension/transport/types.go's +// AbortableInterceptor doc and internal/cmdutil/transport.go's RoundTrip. +const transportAbortPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + "errors" + "net/http" + + "github.com/larksuite/cli/extension/transport" +) + +type abortInterceptor struct{} + +func (abortInterceptor) PreRoundTrip(req *http.Request) func(*http.Response, error) { return nil } + +func (abortInterceptor) PreRoundTripE(req *http.Request) (func(*http.Response, error), error) { + return nil, errors.New("aborted for test") +} + +type abortProvider struct{} + +func (abortProvider) Name() string { return "abort-transport" } +func (abortProvider) ResolveInterceptor(ctx context.Context) transport.Interceptor { + return abortInterceptor{} +} + +func init() { + transport.Register(abortProvider{}) +} +` + +// TestSubsystemTransportAbort pins the transport.AbortableInterceptor offline +// effect: registering a Provider whose PreRoundTripE always errors turns +// every outbound API call into an abort before any network round trip. The +// destination URL embedded in the message varies run-to-run (it depends on +// whether the SDK fetches an OAuth token first or calls the target endpoint +// first), so the assertion is pinned to the stable part only. Observed real +// output for `docs +fetch --doc nonexistent` (a real read-risk network +// command, run twice across separate `go test -count` invocations): +// +// run 1: exit=4 stderr={"ok":false,"identity":"user","error":{"type":"network", +// "subtype":"transport","message":"API call failed: Post +// \"https://open.feishu.cn/open-apis/authen/v2/oauth/token\": extension +// \"abort-transport\" aborted round trip: aborted for test"}} +// run 2: exit=4 stderr={... same shape, message targets +// ".../open-apis/docs_ai/v1/documents/nonexistent/fetch" instead ...} +func TestSubsystemTransportAbort(t *testing.T) { + bin := buildFork(t, "transport-abort", transportAbortPlugin) + res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + if res.exit != 4 { + t.Fatalf("exit=%d want 4; stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "network" { + t.Errorf("error.type=%q want network", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "transport" { + t.Errorf("error.subtype=%q want transport", got) + } + if msg := gjson.Get(res.stderr, "error.message").String(); !strings.Contains(msg, `extension "abort-transport" aborted round trip: aborted for test`) { + t.Errorf("error.message=%q want to contain the abort-transport reason", msg) + } +} + +// credentialBlockPlugin registers a credential.Provider whose ResolveAccount +// (and ResolveToken) unconditionally return a *credential.BlockError. +// internal/credential/credential_provider.go's doResolveAccount returns this +// error straight from the provider loop -- before any defaultAcct fallback +// and, transitively, before the LarkClient/HttpClient phases that would issue +// a real network call ever run (see internal/cmdutil/factory_default.go's +// Phase 2 -> Phase 4 ordering). +const credentialBlockPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. +package plugin + +import ( + "context" + + "github.com/larksuite/cli/extension/credential" +) + +type blockProvider struct{} + +func (blockProvider) Name() string { return "block-cred" } + +func (blockProvider) ResolveAccount(ctx context.Context) (*credential.Account, error) { + return nil, &credential.BlockError{Provider: "block-cred", Reason: "blocked for test"} +} + +func (blockProvider) ResolveToken(ctx context.Context, req credential.TokenSpec) (*credential.Token, error) { + return nil, &credential.BlockError{Provider: "block-cred", Reason: "blocked for test"} +} + +func init() { + credential.Register(blockProvider{}) +} +` + +// TestSubsystemCredentialBlock pins the credential.BlockError offline effect. +// Observed real output for `docs +fetch --doc nonexistent`, run twice across +// separate `go test -count` invocations (byte-identical both times, unlike +// the transport-abort case -- credential resolution happens once, before any +// endpoint is chosen, so there is no varying destination URL to leak into the +// message): +// +// exit=5 +// stdout=(empty) +// stderr={"ok":false,"identity":"bot","error":{"type":"internal","subtype":"unknown", +// "message":"blocked by block-cred: blocked for test"}} +func TestSubsystemCredentialBlock(t *testing.T) { + bin := buildFork(t, "credential-block", credentialBlockPlugin) + res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") + t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + if res.exit != 5 { + t.Fatalf("exit=%d want 5; stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + if !gjson.Valid(res.stderr) { + t.Fatalf("stderr not JSON: %s", res.stderr) + } + if got := gjson.Get(res.stderr, "error.type").String(); got != "internal" { + t.Errorf("error.type=%q want internal", got) + } + if got := gjson.Get(res.stderr, "error.subtype").String(); got != "unknown" { + t.Errorf("error.subtype=%q want unknown", got) + } + if msg := gjson.Get(res.stderr, "error.message").String(); msg != "blocked by block-cred: blocked for test" { + t.Errorf("error.message=%q want %q", msg, "blocked by block-cred: blocked for test") + } +} From 11d1de8c2edff16a9babd900b31d83cf1fb654bc Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 15:01:24 +0800 Subject: [PATCH 07/21] ci: add plugin-integration job and wire results gate --- .github/workflows/ci.yml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41eaa9fc42..8eb34a17e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,19 @@ jobs: exit 1 fi + plugin-integration: + needs: fast-gate + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 + with: + go-version-file: go.mod + # No fetch_meta: the git-archive clean tree must embed only the + # committed meta_data stub (reproduces the bare-module customer state). + - name: Run plugin-integration L4 tests + run: go test -count=1 -timeout=15m ./tests/plugin_e2e/... + # ── Layer 2: Quality Gate ────────────────────────────────────────── unit-test: needs: fast-gate @@ -414,7 +427,7 @@ jobs: # ── Results Gate (single required check for branch protection) ───── results: if: ${{ always() }} - needs: [fast-gate, unit-test, lint, script-test, deterministic-gate, coverage, deadcode, e2e-dry-run, e2e-live, security, license-header] + needs: [fast-gate, unit-test, lint, script-test, deterministic-gate, coverage, deadcode, e2e-dry-run, e2e-live, security, license-header, plugin-integration] runs-on: ubuntu-latest steps: - name: Evaluate results @@ -434,6 +447,7 @@ jobs: echo "| L3 | e2e-live | ${{ needs.e2e-live.result }} |" >> $GITHUB_STEP_SUMMARY echo "| L4 | security | ${{ needs.security.result }} |" >> $GITHUB_STEP_SUMMARY echo "| L4 | license-header | ${{ needs.license-header.result }} |" >> $GITHUB_STEP_SUMMARY + echo "| L4 | plugin-integration | ${{ needs.plugin-integration.result }} |" >> $GITHUB_STEP_SUMMARY # Any failure or cancellation in any job blocks the merge. # Legitimately skipped jobs (deadcode on push, e2e-live on fork, @@ -450,7 +464,8 @@ jobs: "${{ needs.e2e-dry-run.result }}" \ "${{ needs.e2e-live.result }}" \ "${{ needs.security.result }}" \ - "${{ needs.license-header.result }}"; do + "${{ needs.license-header.result }}" \ + "${{ needs.plugin-integration.result }}"; do if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then FAILED=1 fi From d60f71054a25e238da2f945dd396b081f00ab2a0 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 15:24:39 +0800 Subject: [PATCH 08/21] test: add sidecar HMAC round-trip L4 and Makefile tag target --- Makefile | 10 +- tests/sidecar_e2e/roundtrip_test.go | 384 ++++++++++++++++++++++++++++ 2 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 tests/sidecar_e2e/roundtrip_test.go diff --git a/Makefile b/Makefile index aad7eccf25..59e6560bdc 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ PREFIX ?= /usr/local TEST_GOARCH := $(or $(GOARCH),$(shell go env GOARCH)) RACE_FLAG := $(if $(filter riscv64,$(TEST_GOARCH)),,-race) -.PHONY: all build vet fmt-check script-test test unit-test integration-test examples-build quality-gate install uninstall clean fetch_meta gitleaks +.PHONY: all build vet fmt-check script-test test unit-test integration-test examples-build quality-gate install uninstall clean fetch_meta gitleaks sidecar-test all: test @@ -105,6 +105,14 @@ uninstall: clean: rm -f $(BINARY) +# sidecar-test compiles and runs the authsidecar* build-tagged code that the +# default CI matrix never sees (they carry //go:build tags). +sidecar-test: + go build -tags authsidecar -o /dev/null . + go test $(RACE_FLAG) -count=1 -tags authsidecar ./extension/credential/sidecar/ ./extension/transport/sidecar/ ./internal/cmdutil/ + go test $(RACE_FLAG) -count=1 -tags authsidecar_demo ./sidecar/server-demo/ + go test $(RACE_FLAG) -count=1 -tags authsidecar ./tests/sidecar_e2e/ + # Run secret-leak checks locally before pushing. # Step 1: check-doc-tokens catches realistic-looking example tokens in reference # docs and asks you to use _EXAMPLE_TOKEN placeholders instead. diff --git a/tests/sidecar_e2e/roundtrip_test.go b/tests/sidecar_e2e/roundtrip_test.go new file mode 100644 index 0000000000..3476c34d67 --- /dev/null +++ b/tests/sidecar_e2e/roundtrip_test.go @@ -0,0 +1,384 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +// Package sidecar_e2e proves the sidecar auth-proxy wire protocol end-to-end, +// offline and secret-free: a real fork binary (built with -tags authsidecar, +// exercising the REAL extension/transport/sidecar interceptor) signs a +// request with HMAC-SHA256 and routes it to an in-test sidecar, which +// verifies the signature using the REAL sidecar.Verify / sidecar.CanonicalRequest +// from github.com/larksuite/cli/sidecar, injects a synthetic token, and +// forwards to an in-test mock upstream. +// +// DEVIATION FROM THE ORIGINAL PLAN: the plan called for driving the real +// sidecar/server-demo binary (built with -tags authsidecar_demo) as the +// middle process. That is infeasible for an OFFLINE test, for three +// independent reasons, all verified in source: +// +// 1. sidecar/server-demo/handler.go:171 resolves a REAL token via +// h.cred.ResolveToken(...), which errors out unless the machine has run +// `lark-cli auth login` — there is no way to make it return a token +// without live credentials. +// 2. sidecar/server-demo/main.go builds handler.allowedHosts from +// core.ResolveEndpoints(BrandFeishu/BrandLark) only — real feishu/lark +// hosts. An in-test mock (127.0.0.1:) is never in that allowlist +// and would be rejected with 403 (handler.go step 4). +// 3. sidecar/server-demo/handler.go:184 pins the forward scheme to +// "https://" + targetHost, ignoring the client-supplied scheme. It can +// never be redirected to an http:// mock. +// +// server-demo's verify+inject logic is ALREADY covered by +// `go test -tags authsidecar_demo ./sidecar/server-demo/` (see the +// sidecar-test Makefile target, item 3) — that is unit-level coverage of the +// same code paths this file would otherwise exercise via a real subprocess. +// +// So instead, this test builds its OWN in-test sidecar (an httptest.Server) +// that mirrors server-demo/handler.go's verify+inject steps 0-8 exactly, +// using the real protocol package (sidecar.Verify, sidecar.CanonicalRequest, +// sidecar.BodySHA256, the Header* / Sentinel* / Identity* constants) — the +// same symbols server-demo itself uses. This is the standard shape for this +// kind of test: one real external process (the fork binary, compiled with +// the production interceptor code) plus two in-process httptest.Server +// stand-ins (sidecar, upstream). It proves the real wire protocol end-to-end +// without requiring live credentials, real feishu/lark hosts, or TLS. +// +// Every key/token/app-id here is an obviously-synthetic placeholder; nothing +// in this file can authenticate against anything real. +package sidecar_e2e + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/larksuite/cli/sidecar" +) + +// Synthetic, obviously-fake fixtures. None of these are real secrets. +const ( + testProxyKey = "test-proxy-key-not-a-real-secret-000000000000" + testAppID = "cli_test_app_not_real" + injectedToken = "fake-injected-token-not-real" +) + +// capturedRequest snapshots the parts of an *http.Request that matter for +// assertions, taken before the request (and its body reader) is consumed or +// goes out of scope. +type capturedRequest struct { + method string + path string + headers http.Header + body []byte +} + +func snapshotRequest(r *http.Request, body []byte) capturedRequest { + return capturedRequest{ + method: r.Method, + path: r.URL.RequestURI(), + headers: r.Header.Clone(), + body: body, + } +} + +// isProxyHeader reports whether name is one of the sidecar wire-protocol +// headers that must not be copied through to the forwarded (mock upstream) +// request. Mirrors sidecar/server-demo/handler.go's isProxyHeader. +func isProxyHeader(name string) bool { + switch http.CanonicalHeaderKey(name) { + case http.CanonicalHeaderKey(sidecar.HeaderProxyVersion), + http.CanonicalHeaderKey(sidecar.HeaderProxyTarget), + http.CanonicalHeaderKey(sidecar.HeaderProxyIdentity), + http.CanonicalHeaderKey(sidecar.HeaderProxySignature), + http.CanonicalHeaderKey(sidecar.HeaderProxyTimestamp), + http.CanonicalHeaderKey(sidecar.HeaderBodySHA256), + http.CanonicalHeaderKey(sidecar.HeaderProxyAuthHeader): + return true + } + return false +} + +// parseTargetHost validates X-Lark-Proxy-Target and returns its host. +// Mirrors sidecar/server-demo/handler.go's parseTarget: the header must be +// "https://" with no path, query, fragment, or userinfo. Only the host +// is used, both as HMAC signing input and to record what the fork believed +// its real destination was — the actual forward in this test always goes to +// the in-test mock, never to this host. +func parseTargetHost(target string) (string, error) { + u, err := url.Parse(target) + if err != nil { + return "", fmt.Errorf("parse: %w", err) + } + if u.Scheme != "https" { + return "", fmt.Errorf("scheme must be https, got %q", u.Scheme) + } + if u.Host == "" { + return "", fmt.Errorf("missing host") + } + if u.User != nil { + return "", fmt.Errorf("userinfo not allowed") + } + if u.Path != "" && u.Path != "/" { + return "", fmt.Errorf("path not allowed (got %q)", u.Path) + } + if u.RawQuery != "" { + return "", fmt.Errorf("query not allowed") + } + if u.Fragment != "" { + return "", fmt.Errorf("fragment not allowed") + } + return u.Host, nil +} + +// repoRoot resolves the lark-cli module root from the test's working +// directory (which `go test` sets to the package dir, tests/sidecar_e2e). +func repoRoot(t *testing.T) string { + t.Helper() + out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() + if err != nil { + t.Fatalf("resolve repo root: %v", err) + } + return strings.TrimSpace(string(out)) +} + +func TestSidecarHMACRoundTrip(t *testing.T) { + root := repoRoot(t) + sidecarKey := []byte(testProxyKey) + + // --- in-test mock upstream: stands in for open.feishu.cn --- + var mockMu sync.Mutex + var mockReq *capturedRequest + mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + snap := snapshotRequest(r, body) + mockMu.Lock() + mockReq = &snap + mockMu.Unlock() + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"code":0,"msg":"success","data":{"document":{"content":"mock content"}}}`)) + })) + defer mock.Close() + + // --- in-test sidecar: mirrors server-demo/handler.go's verify+inject path --- + var scMu sync.Mutex + var scReq *capturedRequest + var verifyErr error + var verifyRan bool + sc := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + snap := snapshotRequest(r, body) + scMu.Lock() + scReq = &snap + scMu.Unlock() + + // Step 0: protocol version. + version := r.Header.Get(sidecar.HeaderProxyVersion) + if version != sidecar.ProtocolV1 { + http.Error(w, "unsupported "+sidecar.HeaderProxyVersion+": "+version, http.StatusBadRequest) + return + } + + // Step 1-2: timestamp + body SHA256. + ts := r.Header.Get(sidecar.HeaderProxyTimestamp) + claimedSHA := r.Header.Get(sidecar.HeaderBodySHA256) + actualSHA := sidecar.BodySHA256(body) + if claimedSHA == "" || claimedSHA != actualSHA { + http.Error(w, "body SHA256 mismatch", http.StatusBadRequest) + return + } + + // Step 3: target host, identity, auth-header (all covered by the sig). + target := r.Header.Get(sidecar.HeaderProxyTarget) + targetHost, perr := parseTargetHost(target) + if perr != nil { + http.Error(w, "invalid "+sidecar.HeaderProxyTarget+": "+perr.Error(), http.StatusForbidden) + return + } + identity := r.Header.Get(sidecar.HeaderProxyIdentity) + authHeader := r.Header.Get(sidecar.HeaderProxyAuthHeader) + + // Step 4: verify HMAC signature over the canonical request. + signature := r.Header.Get(sidecar.HeaderProxySignature) + err := sidecar.Verify(sidecarKey, sidecar.CanonicalRequest{ + Version: version, + Method: r.Method, + Host: targetHost, + PathAndQuery: r.URL.RequestURI(), + BodySHA256: claimedSHA, + Timestamp: ts, + Identity: identity, + AuthHeader: authHeader, + }, signature) + scMu.Lock() + verifyErr = err + verifyRan = true + scMu.Unlock() + if err != nil { + http.Error(w, "HMAC verification failed: "+err.Error(), http.StatusUnauthorized) + return + } + + // Build the forward request. Unlike server-demo (which forwards to + // "https://"+targetHost), this test forwards to the in-test MOCK's + // URL — proving the sidecar's inject step without needing a real + // upstream or a route to targetHost. + forwardURL := mock.URL + r.URL.RequestURI() + freq, ferr := http.NewRequest(r.Method, forwardURL, bytes.NewReader(body)) + if ferr != nil { + http.Error(w, "failed to build forward request", http.StatusInternalServerError) + return + } + for k, vs := range r.Header { + if isProxyHeader(k) { + continue + } + for _, v := range vs { + freq.Header.Add(k, v) + } + } + // Strip any client-supplied auth headers before injecting (mirrors + // handler.go: the sidecar is the sole source of auth material). + freq.Header.Del("Authorization") + freq.Header.Del(sidecar.HeaderMCPUAT) + freq.Header.Del(sidecar.HeaderMCPTAT) + + // Inject the synthetic token into the header the client committed to. + if authHeader == "Authorization" { + freq.Header.Set("Authorization", "Bearer "+injectedToken) + } else { + freq.Header.Set(authHeader, injectedToken) + } + + resp, derr := http.DefaultClient.Do(freq) + if derr != nil { + http.Error(w, "forward failed: "+derr.Error(), http.StatusBadGateway) + return + } + defer resp.Body.Close() + respBody, _ := io.ReadAll(resp.Body) + for k, vs := range resp.Header { + for _, v := range vs { + w.Header().Add(k, v) + } + } + w.WriteHeader(resp.StatusCode) + _, _ = w.Write(respBody) + })) + defer sc.Close() + + scURL, err := url.Parse(sc.URL) + if err != nil { + t.Fatalf("parse sidecar URL: %v", err) + } + + // --- build the fork: the REAL lark-cli built with -tags authsidecar --- + binPath := filepath.Join(t.TempDir(), "forkbin") + build := exec.Command("go", "build", "-tags", "authsidecar", "-o", binPath, ".") + build.Dir = root + if out, buildErr := build.CombinedOutput(); buildErr != nil { + t.Fatalf("build fork binary: %v\n%s", buildErr, out) + } + + // --- run the fork against the in-test sidecar, fully offline --- + configDir := t.TempDir() + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, binPath, "docs", "+fetch", "--doc", "nonexistent", "--as", "user") + cmd.Env = append(os.Environ(), + "LARKSUITE_CLI_AUTH_PROXY=http://"+scURL.Host, + "LARKSUITE_CLI_PROXY_KEY="+testProxyKey, + "LARKSUITE_CLI_APP_ID="+testAppID, + "LARKSUITE_CLI_BRAND=feishu", + "LARKSUITE_CLI_CONFIG_DIR="+configDir, + "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", + "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + runErr := cmd.Run() + t.Logf("fork exit error (informational only, not asserted): %v", runErr) + t.Logf("fork stdout: %s", stdout.String()) + t.Logf("fork stderr: %s", stderr.String()) + + // --- assertion (a): the sidecar received a request and its HMAC verified --- + scMu.Lock() + gotSC := scReq + gotVerifyRan := verifyRan + gotVerifyErr := verifyErr + scMu.Unlock() + + if gotSC == nil { + t.Fatal("sidecar never received a request from the fork — interceptor did not route to AUTH_PROXY") + } + if !gotVerifyRan { + t.Fatal("sidecar received a request but never reached HMAC verification (rejected earlier — see handler headers)") + } + if gotVerifyErr != nil { + t.Fatalf("HMAC verification failed on the fork's own signed request: %v", gotVerifyErr) + } + t.Logf("fork->sidecar headers: %v", gotSC.headers) + + // --- assertion (c), fork->sidecar half: no real Authorization ever left + // the fork. The interceptor strips the sentinel before signing, so the + // hop the sidecar actually sees must carry no Authorization header (and + // no MCP auth header) at all. + if auth := gotSC.headers.Get("Authorization"); auth != "" { + t.Fatalf("fork->sidecar hop leaked an Authorization header (want none, interceptor should have stripped it): %q", auth) + } + if v := gotSC.headers.Get(sidecar.HeaderMCPUAT); v != "" { + t.Fatalf("fork->sidecar hop leaked %s (want none): %q", sidecar.HeaderMCPUAT, v) + } + if v := gotSC.headers.Get(sidecar.HeaderMCPTAT); v != "" { + t.Fatalf("fork->sidecar hop leaked %s (want none): %q", sidecar.HeaderMCPTAT, v) + } + // Proxy headers must be present (proves the interceptor actually ran). + for _, h := range []string{ + sidecar.HeaderProxyVersion, sidecar.HeaderProxyTarget, sidecar.HeaderProxyIdentity, + sidecar.HeaderProxySignature, sidecar.HeaderProxyTimestamp, sidecar.HeaderBodySHA256, + sidecar.HeaderProxyAuthHeader, + } { + if gotSC.headers.Get(h) == "" { + t.Fatalf("fork->sidecar hop missing required proxy header %s", h) + } + } + if gotSC.headers.Get(sidecar.HeaderProxyIdentity) != sidecar.IdentityUser { + t.Fatalf("fork->sidecar identity = %q, want %q", gotSC.headers.Get(sidecar.HeaderProxyIdentity), sidecar.IdentityUser) + } + + // --- assertion (b): the mock upstream received the sidecar-injected + // synthetic token, proving injection actually happened. + mockMu.Lock() + gotMock := mockReq + mockMu.Unlock() + + if gotMock == nil { + t.Fatal("mock upstream never received a forwarded request — sidecar did not forward after verification") + } + t.Logf("sidecar->mock headers: %v", gotMock.headers) + + wantAuth := "Bearer " + injectedToken + gotAuth := gotMock.headers.Get("Authorization") + if gotAuth != wantAuth { + t.Fatalf("mock upstream Authorization = %q, want %q", gotAuth, wantAuth) + } + // Belt-and-suspenders: the value the mock saw must not be either sentinel + // — proving the only token that ever reached "upstream" was the + // sidecar-injected synthetic one, never a sentinel or a real token. + if gotAuth == "Bearer "+sidecar.SentinelUAT || gotAuth == "Bearer "+sidecar.SentinelTAT { + t.Fatalf("mock upstream received a sentinel token instead of the injected one: %q", gotAuth) + } +} From ecded9975e18ecd9937d5a40d2d9008ad0e0f70b Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 15:40:57 +0800 Subject: [PATCH 09/21] ci: add sidecar-integration job and wire results gate --- .github/workflows/ci.yml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8eb34a17e7..edf3f45a18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,6 +60,17 @@ jobs: - name: Run plugin-integration L4 tests run: go test -count=1 -timeout=15m ./tests/plugin_e2e/... + sidecar-integration: + needs: fast-gate + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 + with: + go-version-file: go.mod + - name: Run sidecar tag build + HMAC round-trip + run: make sidecar-test + # ── Layer 2: Quality Gate ────────────────────────────────────────── unit-test: needs: fast-gate @@ -427,7 +438,7 @@ jobs: # ── Results Gate (single required check for branch protection) ───── results: if: ${{ always() }} - needs: [fast-gate, unit-test, lint, script-test, deterministic-gate, coverage, deadcode, e2e-dry-run, e2e-live, security, license-header, plugin-integration] + needs: [fast-gate, unit-test, lint, script-test, deterministic-gate, coverage, deadcode, e2e-dry-run, e2e-live, security, license-header, plugin-integration, sidecar-integration] runs-on: ubuntu-latest steps: - name: Evaluate results @@ -448,6 +459,7 @@ jobs: echo "| L4 | security | ${{ needs.security.result }} |" >> $GITHUB_STEP_SUMMARY echo "| L4 | license-header | ${{ needs.license-header.result }} |" >> $GITHUB_STEP_SUMMARY echo "| L4 | plugin-integration | ${{ needs.plugin-integration.result }} |" >> $GITHUB_STEP_SUMMARY + echo "| L4 | sidecar-integration | ${{ needs.sidecar-integration.result }} |" >> $GITHUB_STEP_SUMMARY # Any failure or cancellation in any job blocks the merge. # Legitimately skipped jobs (deadcode on push, e2e-live on fork, @@ -465,7 +477,8 @@ jobs: "${{ needs.e2e-live.result }}" \ "${{ needs.security.result }}" \ "${{ needs.license-header.result }}" \ - "${{ needs.plugin-integration.result }}"; do + "${{ needs.plugin-integration.result }}" \ + "${{ needs.sidecar-integration.result }}"; do if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then FAILED=1 fi From 1052c9cdbba6bffdc12fd19bc0e717221ff19f72 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 16:32:37 +0800 Subject: [PATCH 10/21] ci: make plugin/sidecar integration jobs observe-only The two new L4 jobs (plugin-integration, sidecar-integration) still run on every PR and report status in the results summary, but their failure no longer blocks the merge gate during the initial soak. Once they prove stable, add them back to the results FAILED loop to make them required. --- .github/workflows/ci.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index edf3f45a18..ecdb3c615e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -458,12 +458,18 @@ jobs: echo "| L3 | e2e-live | ${{ needs.e2e-live.result }} |" >> $GITHUB_STEP_SUMMARY echo "| L4 | security | ${{ needs.security.result }} |" >> $GITHUB_STEP_SUMMARY echo "| L4 | license-header | ${{ needs.license-header.result }} |" >> $GITHUB_STEP_SUMMARY - echo "| L4 | plugin-integration | ${{ needs.plugin-integration.result }} |" >> $GITHUB_STEP_SUMMARY - echo "| L4 | sidecar-integration | ${{ needs.sidecar-integration.result }} |" >> $GITHUB_STEP_SUMMARY + echo "| L4 | plugin-integration (observe-only) | ${{ needs.plugin-integration.result }} |" >> $GITHUB_STEP_SUMMARY + echo "| L4 | sidecar-integration (observe-only) | ${{ needs.sidecar-integration.result }} |" >> $GITHUB_STEP_SUMMARY # Any failure or cancellation in any job blocks the merge. # Legitimately skipped jobs (deadcode on push, e2e-live on fork, # license-header on push) are OK. + # + # plugin-integration and sidecar-integration are intentionally NOT + # in this loop yet: they run on every PR and their status is shown + # in the table above, but a failure is observe-only (non-blocking) + # during the initial soak. Add them back here to make them required + # once they have proven stable. FAILED=0 for result in \ "${{ needs.fast-gate.result }}" \ @@ -476,9 +482,7 @@ jobs: "${{ needs.e2e-dry-run.result }}" \ "${{ needs.e2e-live.result }}" \ "${{ needs.security.result }}" \ - "${{ needs.license-header.result }}" \ - "${{ needs.plugin-integration.result }}" \ - "${{ needs.sidecar-integration.result }}"; do + "${{ needs.license-header.result }}"; do if [ "$result" = "failure" ] || [ "$result" = "cancelled" ]; then FAILED=1 fi From 78f8837567a1114371ab28d29456b06f0c53d516 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 8 Jul 2026 17:43:55 +0800 Subject: [PATCH 11/21] test: dedupe plugin_e2e envelope assert and fix comments Merge the byte-identical assertInstallEnvelope into a single neutral assertReasonCodeEnvelope shared by both policy-denial and install-time reason_code tests. Reword the observer-panic-isolation comment to describe the baseline-relative assertion instead of a hardcoded exit 0, and the auditPlugin comment to say it is based on (not mirrors) the shipped example. --- tests/plugin_e2e/install_test.go | 42 +++++---------------------- tests/plugin_e2e/observe_wrap_test.go | 17 ++++++----- tests/plugin_e2e/restrict_test.go | 18 +++++++----- 3 files changed, 27 insertions(+), 50 deletions(-) diff --git a/tests/plugin_e2e/install_test.go b/tests/plugin_e2e/install_test.go index 2ef5fdba3b..812e2dbff3 100644 --- a/tests/plugin_e2e/install_test.go +++ b/tests/plugin_e2e/install_test.go @@ -10,34 +10,6 @@ import ( "github.com/tidwall/gjson" ) -// assertInstallEnvelope asserts the VERIFIED install-time failure envelope -// shape shared by every reason_code in this file: exit 2, valid JSON on -// stderr, error.type=="validation", error.subtype=="failed_precondition", -// and error.hint containing "reason_code ". This mirrors -// assertDenialEnvelope in restrict_test.go -- install-time failures render -// through the SAME cmd/platform_guards.go WithHint(...) family as policy -// denials, embedding reason_code in the hint STRING, not a structured -// error.detail.reason_code field (contradicting the candidate shape -// referenced from internal/platform/error.go:34's comment). -func assertInstallEnvelope(t *testing.T, res result, wantReasonCode string) { - t.Helper() - if res.exit != 2 { - t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - } - if !gjson.Valid(res.stderr) { - t.Fatalf("stderr not JSON: %s", res.stderr) - } - if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { - t.Errorf("error.type=%q want validation", got) - } - if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { - t.Errorf("error.subtype=%q want failed_precondition", got) - } - if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, "reason_code "+wantReasonCode) { - t.Errorf("hint=%q want to contain reason_code %s", hint, wantReasonCode) - } -} - // multipleRestrictPlugin registers TWO distinct plugins that each call // Restrict() with an independently valid Rule. cmdpolicy.Resolve rejects // more than one distinct Restrict-owner regardless of each rule's own @@ -83,7 +55,7 @@ func TestInstallMultipleRestrictPluginsPin(t *testing.T) { bin := buildFork(t, "multiple-restrict", multipleRestrictPlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "multiple_restrict_plugins") + assertReasonCodeEnvelope(t, res, "multiple_restrict_plugins") } // invalidRulePlugin registers a single plugin whose Restrict Rule carries a @@ -121,7 +93,7 @@ func TestInstallInvalidRulePin(t *testing.T) { bin := buildFork(t, "invalid-rule", invalidRulePlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "invalid_rule") + assertReasonCodeEnvelope(t, res, "invalid_rule") } // installFailedPlugin is a hand-written bare platform.Plugin (not @@ -164,7 +136,7 @@ func TestInstallFailedPin(t *testing.T) { bin := buildFork(t, "install-failed", installFailedPlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "install_failed") + assertReasonCodeEnvelope(t, res, "install_failed") } // installPanicPlugin is a hand-written bare Plugin whose Install panics. @@ -202,7 +174,7 @@ func TestInstallPanicPin(t *testing.T) { bin := buildFork(t, "install-panic", installPanicPlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "install_panic") + assertReasonCodeEnvelope(t, res, "install_panic") } // pluginNamePanicPlugin is a hand-written bare Plugin whose Name() panics. @@ -240,7 +212,7 @@ func TestInstallPluginNamePanicPin(t *testing.T) { bin := buildFork(t, "plugin-name-panic", pluginNamePanicPlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "plugin_name_panic") + assertReasonCodeEnvelope(t, res, "plugin_name_panic") } // capabilitiesPanicPlugin is a hand-written bare Plugin whose Capabilities() @@ -278,7 +250,7 @@ func TestInstallCapabilitiesPanicPin(t *testing.T) { bin := buildFork(t, "capabilities-panic", capabilitiesPanicPlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "capabilities_panic") + assertReasonCodeEnvelope(t, res, "capabilities_panic") } // restrictsMismatchPlugin is a hand-written bare Plugin that declares @@ -316,7 +288,7 @@ func TestInstallRestrictsMismatchPin(t *testing.T) { bin := buildFork(t, "restricts-mismatch", restrictsMismatchPlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertInstallEnvelope(t, res, "restricts_mismatch") + assertReasonCodeEnvelope(t, res, "restricts_mismatch") } // mustBuildPanicPlugin calls MustBuild() on a Builder with an invalid plugin diff --git a/tests/plugin_e2e/observe_wrap_test.go b/tests/plugin_e2e/observe_wrap_test.go index 46386fb8b8..64a998a478 100644 --- a/tests/plugin_e2e/observe_wrap_test.go +++ b/tests/plugin_e2e/observe_wrap_test.go @@ -11,8 +11,8 @@ import ( ) // auditPlugin registers a single After observer matching every command that -// logs "[audit] " to stderr. Mirrors the shipped -// extension/platform/examples/audit-observer example. +// logs "[audit] " to stderr. Based on (a simplified form of) the +// shipped extension/platform/examples/audit-observer example. const auditPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. package plugin @@ -151,16 +151,17 @@ func init() { ` // TestObserverPanicIsolationPin pins panic isolation: an After observer that -// always panics must not affect the command's own outcome. Observed real -// output for `schema` (a local, network-free, read-risk command) under the -// panicking-observer fork vs. the noop-observer baseline fork: +// always panics must not affect the command's own outcome. The assertion is +// baseline-relative -- the panicking-observer fork's exit code must equal the +// noop-observer baseline fork's for the same `schema` command (a local, +// network-free, read-risk command), whatever that shared exit code is. +// Observed real output at pin time: // // panicking: exit=0 stderr=warning: hook "observer-panic.log" panicked: boom // baseline: exit=0 stderr=(empty) // -// Both exit 0 identically; the panic is fully swallowed by -// runObserverSafe (internal/hook/install.go), surfacing only as a stderr -// warning line, never as a non-zero exit or crash. +// The panic is fully swallowed by runObserverSafe (internal/hook/install.go), +// surfacing only as a stderr warning line, never as a non-zero exit or crash. func TestObserverPanicIsolationPin(t *testing.T) { bin := buildFork(t, "observer-panic", observerPanicPlugin) res := run(t, bin, "schema") diff --git a/tests/plugin_e2e/restrict_test.go b/tests/plugin_e2e/restrict_test.go index d1897d54b4..112c029f66 100644 --- a/tests/plugin_e2e/restrict_test.go +++ b/tests/plugin_e2e/restrict_test.go @@ -157,11 +157,15 @@ func init() { } ` -// assertDenialEnvelope asserts the VERIFIED denial envelope shape shared by -// every reason_code in this file: exit 2, valid JSON on stderr, +// assertReasonCodeEnvelope asserts the VERIFIED envelope shape shared by every +// reason_code across this package -- both policy denials (this file) and +// install-time failures (install_test.go): exit 2, valid JSON on stderr, // error.type=="validation", error.subtype=="failed_precondition", and -// error.hint containing "reason_code ". -func assertDenialEnvelope(t *testing.T, res result, wantReasonCode string) { +// error.hint containing "reason_code ". Both paths render +// through the SAME cmd/platform_guards.go WithHint(...) family, embedding +// reason_code in the hint STRING, not a structured error.detail.reason_code +// field (contradicting internal/platform/error.go:34's comment). +func assertReasonCodeEnvelope(t *testing.T, res result, wantReasonCode string) { t.Helper() if res.exit != 2 { t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) @@ -186,7 +190,7 @@ func TestIdentityMismatchDenial(t *testing.T) { bin := buildFork(t, "identity", identityPlugin) res := run(t, bin, "im", "+messages-search", "--as", "user") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertDenialEnvelope(t, res, "identity_mismatch") + assertReasonCodeEnvelope(t, res, "identity_mismatch") } // TestDenylistDenial pins reason_code=command_denylisted: a Deny glob hit @@ -195,7 +199,7 @@ func TestDenylistDenial(t *testing.T) { bin := buildFork(t, "denylist", denylistPlugin) res := run(t, bin, "docs", "+search") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertDenialEnvelope(t, res, "command_denylisted") + assertReasonCodeEnvelope(t, res, "command_denylisted") } // TestMultiRuleDenial pins reason_code=no_matching_rule: a command rejected @@ -205,5 +209,5 @@ func TestMultiRuleDenial(t *testing.T) { bin := buildFork(t, "multirule", multiRulePlugin) res := run(t, bin, "schema") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - assertDenialEnvelope(t, res, "no_matching_rule") + assertReasonCodeEnvelope(t, res, "no_matching_rule") } From 407c484ef27c599659130daf24921506b2ff4468 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Thu, 9 Jul 2026 11:47:11 +0800 Subject: [PATCH 12/21] test: split sidecar roundtrip into readable named steps Extract the 230-line TestSidecarHMACRoundTrip into a short top-level flow (start stubs -> build+run fork -> assert) backed by focused helpers: the in-test sidecar handler splits into verifyProxyRequest (steps 0-4) and forwardWithInjectedToken; the two httptest stand-ins become mockUpstream / inTestSidecar types sharing a requestSink; build/run/assert move into buildAuthsidecarFork, runFork, and the two assert* helpers. Behavior unchanged. --- tests/sidecar_e2e/roundtrip_test.go | 444 ++++++++++++++++------------ 1 file changed, 263 insertions(+), 181 deletions(-) diff --git a/tests/sidecar_e2e/roundtrip_test.go b/tests/sidecar_e2e/roundtrip_test.go index 3476c34d67..6a002b3063 100644 --- a/tests/sidecar_e2e/roundtrip_test.go +++ b/tests/sidecar_e2e/roundtrip_test.go @@ -73,6 +73,27 @@ const ( injectedToken = "fake-injected-token-not-real" ) +// TestSidecarHMACRoundTrip drives the whole wire protocol as three named +// steps so the flow is readable at a glance; each step's mechanics live in a +// dedicated helper below. +func TestSidecarHMACRoundTrip(t *testing.T) { + // Two in-process stand-ins: the mock upstream (for open.feishu.cn) and the + // in-test sidecar (server-demo's verify+inject, via the real protocol pkg). + upstream := startMockUpstream(t) + sc := startInTestSidecar(t, []byte(testProxyKey), upstream.URL) + + // One real external process: lark-cli built with -tags authsidecar, run + // fully offline against the in-test sidecar. + bin := buildAuthsidecarFork(t) + runFork(t, bin, sc.URL) + + // Assert the three properties of a correct round trip. + assertInterceptorSigned(t, sc) // (a)+(c) fork -> sidecar + assertInjectedTokenReachedUpstream(t, upstream) // (b) sidecar -> upstream +} + +// --- request capture ------------------------------------------------------- + // capturedRequest snapshots the parts of an *http.Request that matter for // assertions, taken before the request (and its body reader) is consumed or // goes out of scope. @@ -83,13 +104,188 @@ type capturedRequest struct { body []byte } -func snapshotRequest(r *http.Request, body []byte) capturedRequest { - return capturedRequest{ +// requestSink stores the request a stub server saw, guarded so the httptest +// handler goroutine and the test goroutine can hand it over safely. +type requestSink struct { + mu sync.Mutex + req *capturedRequest +} + +func (s *requestSink) capture(r *http.Request, body []byte) { + snap := capturedRequest{ method: r.Method, path: r.URL.RequestURI(), headers: r.Header.Clone(), body: body, } + s.mu.Lock() + s.req = &snap + s.mu.Unlock() +} + +func (s *requestSink) get() *capturedRequest { + s.mu.Lock() + defer s.mu.Unlock() + return s.req +} + +// --- mock upstream (stands in for open.feishu.cn) -------------------------- + +type mockUpstream struct { + *httptest.Server + sink requestSink +} + +func startMockUpstream(t *testing.T) *mockUpstream { + t.Helper() + m := &mockUpstream{} + m.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + m.sink.capture(r, body) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"code":0,"msg":"success","data":{"document":{"content":"mock content"}}}`)) + })) + t.Cleanup(m.Close) + return m +} + +// --- in-test sidecar (mirrors server-demo/handler.go verify+inject) -------- + +type inTestSidecar struct { + *httptest.Server + key []byte + upstreamURL string + sink requestSink + + mu sync.Mutex // guards verifyRan/verifyErr + verifyRan bool + verifyErr error +} + +func startInTestSidecar(t *testing.T, key []byte, upstreamURL string) *inTestSidecar { + t.Helper() + s := &inTestSidecar{key: key, upstreamURL: upstreamURL} + s.Server = httptest.NewServer(http.HandlerFunc(s.handle)) + t.Cleanup(s.Close) + return s +} + +// handle is the request flow: capture -> verify (steps 0-4) -> inject+forward. +func (s *inTestSidecar) handle(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + s.sink.capture(r, body) + + authHeader, ok := s.verifyProxyRequest(w, r, body) + if !ok { + return + } + s.forwardWithInjectedToken(w, r, body, authHeader) +} + +// verifyProxyRequest mirrors server-demo/handler.go steps 0-4: protocol +// version, body SHA256, target validation, and HMAC signature verification. +// It records whether verification ran and its result (for assertions) and +// returns the auth header the client committed to. On any failure it writes +// the HTTP error and returns ok=false. +func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Request, body []byte) (authHeader string, ok bool) { + // Step 0: protocol version. + version := r.Header.Get(sidecar.HeaderProxyVersion) + if version != sidecar.ProtocolV1 { + http.Error(w, "unsupported "+sidecar.HeaderProxyVersion+": "+version, http.StatusBadRequest) + return "", false + } + + // Step 1-2: timestamp + body SHA256. + ts := r.Header.Get(sidecar.HeaderProxyTimestamp) + claimedSHA := r.Header.Get(sidecar.HeaderBodySHA256) + if claimedSHA == "" || claimedSHA != sidecar.BodySHA256(body) { + http.Error(w, "body SHA256 mismatch", http.StatusBadRequest) + return "", false + } + + // Step 3: target host, identity, auth-header (all covered by the sig). + targetHost, perr := parseTargetHost(r.Header.Get(sidecar.HeaderProxyTarget)) + if perr != nil { + http.Error(w, "invalid "+sidecar.HeaderProxyTarget+": "+perr.Error(), http.StatusForbidden) + return "", false + } + identity := r.Header.Get(sidecar.HeaderProxyIdentity) + authHeader = r.Header.Get(sidecar.HeaderProxyAuthHeader) + + // Step 4: verify HMAC signature over the canonical request. + err := sidecar.Verify(s.key, sidecar.CanonicalRequest{ + Version: version, + Method: r.Method, + Host: targetHost, + PathAndQuery: r.URL.RequestURI(), + BodySHA256: claimedSHA, + Timestamp: ts, + Identity: identity, + AuthHeader: authHeader, + }, r.Header.Get(sidecar.HeaderProxySignature)) + s.mu.Lock() + s.verifyRan = true + s.verifyErr = err + s.mu.Unlock() + if err != nil { + http.Error(w, "HMAC verification failed: "+err.Error(), http.StatusUnauthorized) + return "", false + } + return authHeader, true +} + +// forwardWithInjectedToken mirrors server-demo's inject+forward. Unlike +// server-demo (which forwards to "https://"+targetHost), this test forwards to +// the in-test MOCK's URL — proving the sidecar's inject step without needing a +// real upstream or a route to targetHost. It strips any client-supplied auth +// headers first (the sidecar is the sole source of auth material), injects the +// synthetic token into the committed header, and relays the response back. +func (s *inTestSidecar) forwardWithInjectedToken(w http.ResponseWriter, r *http.Request, body []byte, authHeader string) { + freq, err := http.NewRequest(r.Method, s.upstreamURL+r.URL.RequestURI(), bytes.NewReader(body)) + if err != nil { + http.Error(w, "failed to build forward request", http.StatusInternalServerError) + return + } + for k, vs := range r.Header { + if isProxyHeader(k) { + continue + } + for _, v := range vs { + freq.Header.Add(k, v) + } + } + freq.Header.Del("Authorization") + freq.Header.Del(sidecar.HeaderMCPUAT) + freq.Header.Del(sidecar.HeaderMCPTAT) + + if authHeader == "Authorization" { + freq.Header.Set("Authorization", "Bearer "+injectedToken) + } else { + freq.Header.Set(authHeader, injectedToken) + } + + resp, err := http.DefaultClient.Do(freq) + if err != nil { + http.Error(w, "forward failed: "+err.Error(), http.StatusBadGateway) + return + } + defer resp.Body.Close() + respBody, _ := io.ReadAll(resp.Body) + for k, vs := range resp.Header { + for _, v := range vs { + w.Header().Add(k, v) + } + } + w.WriteHeader(resp.StatusCode) + _, _ = w.Write(respBody) +} + +// verifyResult reports whether step 4 ran and, if so, its error. +func (s *inTestSidecar) verifyResult() (ran bool, err error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.verifyRan, s.verifyErr } // isProxyHeader reports whether name is one of the sidecar wire-protocol @@ -141,159 +337,31 @@ func parseTargetHost(target string) (string, error) { return u.Host, nil } -// repoRoot resolves the lark-cli module root from the test's working -// directory (which `go test` sets to the package dir, tests/sidecar_e2e). -func repoRoot(t *testing.T) string { +// --- fork build + run ------------------------------------------------------ + +// buildAuthsidecarFork builds the REAL lark-cli with -tags authsidecar (the +// production interceptor) and returns the binary path. +func buildAuthsidecarFork(t *testing.T) string { t.Helper() - out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() - if err != nil { - t.Fatalf("resolve repo root: %v", err) + bin := filepath.Join(t.TempDir(), "forkbin") + build := exec.Command("go", "build", "-tags", "authsidecar", "-o", bin, ".") + build.Dir = repoRoot(t) + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("build fork binary: %v\n%s", err, out) } - return strings.TrimSpace(string(out)) + return bin } -func TestSidecarHMACRoundTrip(t *testing.T) { - root := repoRoot(t) - sidecarKey := []byte(testProxyKey) - - // --- in-test mock upstream: stands in for open.feishu.cn --- - var mockMu sync.Mutex - var mockReq *capturedRequest - mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - snap := snapshotRequest(r, body) - mockMu.Lock() - mockReq = &snap - mockMu.Unlock() - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"code":0,"msg":"success","data":{"document":{"content":"mock content"}}}`)) - })) - defer mock.Close() - - // --- in-test sidecar: mirrors server-demo/handler.go's verify+inject path --- - var scMu sync.Mutex - var scReq *capturedRequest - var verifyErr error - var verifyRan bool - sc := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - snap := snapshotRequest(r, body) - scMu.Lock() - scReq = &snap - scMu.Unlock() - - // Step 0: protocol version. - version := r.Header.Get(sidecar.HeaderProxyVersion) - if version != sidecar.ProtocolV1 { - http.Error(w, "unsupported "+sidecar.HeaderProxyVersion+": "+version, http.StatusBadRequest) - return - } - - // Step 1-2: timestamp + body SHA256. - ts := r.Header.Get(sidecar.HeaderProxyTimestamp) - claimedSHA := r.Header.Get(sidecar.HeaderBodySHA256) - actualSHA := sidecar.BodySHA256(body) - if claimedSHA == "" || claimedSHA != actualSHA { - http.Error(w, "body SHA256 mismatch", http.StatusBadRequest) - return - } - - // Step 3: target host, identity, auth-header (all covered by the sig). - target := r.Header.Get(sidecar.HeaderProxyTarget) - targetHost, perr := parseTargetHost(target) - if perr != nil { - http.Error(w, "invalid "+sidecar.HeaderProxyTarget+": "+perr.Error(), http.StatusForbidden) - return - } - identity := r.Header.Get(sidecar.HeaderProxyIdentity) - authHeader := r.Header.Get(sidecar.HeaderProxyAuthHeader) - - // Step 4: verify HMAC signature over the canonical request. - signature := r.Header.Get(sidecar.HeaderProxySignature) - err := sidecar.Verify(sidecarKey, sidecar.CanonicalRequest{ - Version: version, - Method: r.Method, - Host: targetHost, - PathAndQuery: r.URL.RequestURI(), - BodySHA256: claimedSHA, - Timestamp: ts, - Identity: identity, - AuthHeader: authHeader, - }, signature) - scMu.Lock() - verifyErr = err - verifyRan = true - scMu.Unlock() - if err != nil { - http.Error(w, "HMAC verification failed: "+err.Error(), http.StatusUnauthorized) - return - } - - // Build the forward request. Unlike server-demo (which forwards to - // "https://"+targetHost), this test forwards to the in-test MOCK's - // URL — proving the sidecar's inject step without needing a real - // upstream or a route to targetHost. - forwardURL := mock.URL + r.URL.RequestURI() - freq, ferr := http.NewRequest(r.Method, forwardURL, bytes.NewReader(body)) - if ferr != nil { - http.Error(w, "failed to build forward request", http.StatusInternalServerError) - return - } - for k, vs := range r.Header { - if isProxyHeader(k) { - continue - } - for _, v := range vs { - freq.Header.Add(k, v) - } - } - // Strip any client-supplied auth headers before injecting (mirrors - // handler.go: the sidecar is the sole source of auth material). - freq.Header.Del("Authorization") - freq.Header.Del(sidecar.HeaderMCPUAT) - freq.Header.Del(sidecar.HeaderMCPTAT) - - // Inject the synthetic token into the header the client committed to. - if authHeader == "Authorization" { - freq.Header.Set("Authorization", "Bearer "+injectedToken) - } else { - freq.Header.Set(authHeader, injectedToken) - } - - resp, derr := http.DefaultClient.Do(freq) - if derr != nil { - http.Error(w, "forward failed: "+derr.Error(), http.StatusBadGateway) - return - } - defer resp.Body.Close() - respBody, _ := io.ReadAll(resp.Body) - for k, vs := range resp.Header { - for _, v := range vs { - w.Header().Add(k, v) - } - } - w.WriteHeader(resp.StatusCode) - _, _ = w.Write(respBody) - })) - defer sc.Close() - - scURL, err := url.Parse(sc.URL) +// runFork runs the fork against the in-test sidecar, fully offline. The fork's +// exit status is logged but NOT asserted — this test judges wire behavior +// (what reached the sidecar/upstream), not the command's own success. +func runFork(t *testing.T, binPath, sidecarURL string) { + t.Helper() + scURL, err := url.Parse(sidecarURL) if err != nil { t.Fatalf("parse sidecar URL: %v", err) } - // --- build the fork: the REAL lark-cli built with -tags authsidecar --- - binPath := filepath.Join(t.TempDir(), "forkbin") - build := exec.Command("go", "build", "-tags", "authsidecar", "-o", binPath, ".") - build.Dir = root - if out, buildErr := build.CombinedOutput(); buildErr != nil { - t.Fatalf("build fork binary: %v\n%s", buildErr, out) - } - - // --- run the fork against the in-test sidecar, fully offline --- - configDir := t.TempDir() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() cmd := exec.CommandContext(ctx, binPath, "docs", "+fetch", "--doc", "nonexistent", "--as", "user") @@ -302,7 +370,7 @@ func TestSidecarHMACRoundTrip(t *testing.T) { "LARKSUITE_CLI_PROXY_KEY="+testProxyKey, "LARKSUITE_CLI_APP_ID="+testAppID, "LARKSUITE_CLI_BRAND=feishu", - "LARKSUITE_CLI_CONFIG_DIR="+configDir, + "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", ) @@ -313,71 +381,85 @@ func TestSidecarHMACRoundTrip(t *testing.T) { t.Logf("fork exit error (informational only, not asserted): %v", runErr) t.Logf("fork stdout: %s", stdout.String()) t.Logf("fork stderr: %s", stderr.String()) +} - // --- assertion (a): the sidecar received a request and its HMAC verified --- - scMu.Lock() - gotSC := scReq - gotVerifyRan := verifyRan - gotVerifyErr := verifyErr - scMu.Unlock() +// repoRoot resolves the lark-cli module root from the test's working +// directory (which `go test` sets to the package dir, tests/sidecar_e2e). +func repoRoot(t *testing.T) string { + t.Helper() + out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() + if err != nil { + t.Fatalf("resolve repo root: %v", err) + } + return strings.TrimSpace(string(out)) +} - if gotSC == nil { +// --- assertions ------------------------------------------------------------ + +// assertInterceptorSigned checks the fork -> sidecar hop (assertions a + c): +// the real interceptor ran (all proxy headers present, identity=user), stripped +// every real/sentinel auth header before signing, and produced a signature that +// verified against the shared key. +func assertInterceptorSigned(t *testing.T, sc *inTestSidecar) { + t.Helper() + got := sc.sink.get() + if got == nil { t.Fatal("sidecar never received a request from the fork — interceptor did not route to AUTH_PROXY") } - if !gotVerifyRan { + ran, verifyErr := sc.verifyResult() + if !ran { t.Fatal("sidecar received a request but never reached HMAC verification (rejected earlier — see handler headers)") } - if gotVerifyErr != nil { - t.Fatalf("HMAC verification failed on the fork's own signed request: %v", gotVerifyErr) + if verifyErr != nil { + t.Fatalf("HMAC verification failed on the fork's own signed request: %v", verifyErr) } - t.Logf("fork->sidecar headers: %v", gotSC.headers) + t.Logf("fork->sidecar headers: %v", got.headers) - // --- assertion (c), fork->sidecar half: no real Authorization ever left - // the fork. The interceptor strips the sentinel before signing, so the - // hop the sidecar actually sees must carry no Authorization header (and - // no MCP auth header) at all. - if auth := gotSC.headers.Get("Authorization"); auth != "" { + // No real/sentinel auth ever left the fork: the interceptor strips the + // sentinel before signing, so this hop must carry no auth header at all. + if auth := got.headers.Get("Authorization"); auth != "" { t.Fatalf("fork->sidecar hop leaked an Authorization header (want none, interceptor should have stripped it): %q", auth) } - if v := gotSC.headers.Get(sidecar.HeaderMCPUAT); v != "" { + if v := got.headers.Get(sidecar.HeaderMCPUAT); v != "" { t.Fatalf("fork->sidecar hop leaked %s (want none): %q", sidecar.HeaderMCPUAT, v) } - if v := gotSC.headers.Get(sidecar.HeaderMCPTAT); v != "" { + if v := got.headers.Get(sidecar.HeaderMCPTAT); v != "" { t.Fatalf("fork->sidecar hop leaked %s (want none): %q", sidecar.HeaderMCPTAT, v) } + // Proxy headers must be present (proves the interceptor actually ran). for _, h := range []string{ sidecar.HeaderProxyVersion, sidecar.HeaderProxyTarget, sidecar.HeaderProxyIdentity, sidecar.HeaderProxySignature, sidecar.HeaderProxyTimestamp, sidecar.HeaderBodySHA256, sidecar.HeaderProxyAuthHeader, } { - if gotSC.headers.Get(h) == "" { + if got.headers.Get(h) == "" { t.Fatalf("fork->sidecar hop missing required proxy header %s", h) } } - if gotSC.headers.Get(sidecar.HeaderProxyIdentity) != sidecar.IdentityUser { - t.Fatalf("fork->sidecar identity = %q, want %q", gotSC.headers.Get(sidecar.HeaderProxyIdentity), sidecar.IdentityUser) + if id := got.headers.Get(sidecar.HeaderProxyIdentity); id != sidecar.IdentityUser { + t.Fatalf("fork->sidecar identity = %q, want %q", id, sidecar.IdentityUser) } +} - // --- assertion (b): the mock upstream received the sidecar-injected - // synthetic token, proving injection actually happened. - mockMu.Lock() - gotMock := mockReq - mockMu.Unlock() - - if gotMock == nil { +// assertInjectedTokenReachedUpstream checks the sidecar -> upstream hop +// (assertion b): the mock saw exactly the sidecar-injected synthetic token, +// never a sentinel or a real one — proving injection actually happened. +func assertInjectedTokenReachedUpstream(t *testing.T, up *mockUpstream) { + t.Helper() + got := up.sink.get() + if got == nil { t.Fatal("mock upstream never received a forwarded request — sidecar did not forward after verification") } - t.Logf("sidecar->mock headers: %v", gotMock.headers) + t.Logf("sidecar->mock headers: %v", got.headers) wantAuth := "Bearer " + injectedToken - gotAuth := gotMock.headers.Get("Authorization") + gotAuth := got.headers.Get("Authorization") if gotAuth != wantAuth { t.Fatalf("mock upstream Authorization = %q, want %q", gotAuth, wantAuth) } - // Belt-and-suspenders: the value the mock saw must not be either sentinel - // — proving the only token that ever reached "upstream" was the - // sidecar-injected synthetic one, never a sentinel or a real token. + // Belt-and-suspenders: the value the mock saw must not be either sentinel, + // proving the only token that ever reached "upstream" was the injected one. if gotAuth == "Bearer "+sidecar.SentinelUAT || gotAuth == "Bearer "+sidecar.SentinelTAT { t.Fatalf("mock upstream received a sentinel token instead of the injected one: %q", gotAuth) } From 15eaaaeccc10d0b93bafb7fc8dc6cace66f7ec7f Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Thu, 9 Jul 2026 19:08:37 +0800 Subject: [PATCH 13/21] test: reword transport-abort comment to drop endpoint path --- tests/plugin_e2e/degrade_subsystem_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/plugin_e2e/degrade_subsystem_test.go b/tests/plugin_e2e/degrade_subsystem_test.go index 772f097081..cecf804432 100644 --- a/tests/plugin_e2e/degrade_subsystem_test.go +++ b/tests/plugin_e2e/degrade_subsystem_test.go @@ -189,8 +189,8 @@ func init() { // "subtype":"transport","message":"API call failed: Post // \"https://open.feishu.cn/open-apis/authen/v2/oauth/token\": extension // \"abort-transport\" aborted round trip: aborted for test"}} -// run 2: exit=4 stderr={... same shape, message targets -// ".../open-apis/docs_ai/v1/documents/nonexistent/fetch" instead ...} +// run 2: exit=4 stderr={... same shape, message targets the docs fetch +// endpoint instead of the OAuth token URL ...} func TestSubsystemTransportAbort(t *testing.T) { bin := buildFork(t, "transport-abort", transportAbortPlugin) res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") From eee38bb96f308ed6fc1fbd39aa6e05058f6302cc Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Thu, 9 Jul 2026 20:18:22 +0800 Subject: [PATCH 14/21] test: make plugin_e2e forks deterministic offline; cover runtime schema catalog Two plugin_e2e tests resolved differently on a clean CI runner than on a developer box because they used run(), which inherits the host's ~/.lark-cli metadata cache and network. On CI (cold cache, offline) they failed; locally they passed. - Replace TestSubsystemTransportAbort with TestRuntimeCatalogResolvesSchema. The transport round-trip it targeted is unreachable in a bare-module fork offline (credential resolution fails first); the transport path is covered end-to-end by the sidecar suite and the credential-block test. The new test seeds an obviously-synthetic service into a bare-module fork's on-disk cache and asserts `schema` resolves it from the runtime catalog -- the primary fix for #1764 (module builds now consult the runtime catalog instead of returning "Unknown service"). It complements the existing cold-cache degrade test, pinning both branches, and runs fully offline. - Drop the readonly parent-group sub-case: mixed_children_policy needs a metadata-enumerated child tree the bare-module fork does not have, so offline it collapses to domain_not_allowed; that reason_code is covered by cmdpolicy unit tests. - Fix errorlint in run() (errors.As), and stream `git archive` into `tar` through an explicit pipe instead of a shell in gitArchive. - Set persist-credentials: false on the two new job checkouts and reuse assertReasonCodeEnvelope in TestReadonlyDenial. --- .github/workflows/ci.yml | 4 + tests/plugin_e2e/degrade_subsystem_test.go | 194 ++++++++++++--------- tests/plugin_e2e/harness.go | 54 +++--- tests/plugin_e2e/restrict_test.go | 26 +-- 4 files changed, 156 insertions(+), 122 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecdb3c615e..db457c4c45 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: false - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 with: go-version-file: go.mod @@ -65,6 +67,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: false - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 with: go-version-file: go.mod diff --git a/tests/plugin_e2e/degrade_subsystem_test.go b/tests/plugin_e2e/degrade_subsystem_test.go index cecf804432..40472bb736 100644 --- a/tests/plugin_e2e/degrade_subsystem_test.go +++ b/tests/plugin_e2e/degrade_subsystem_test.go @@ -5,8 +5,11 @@ package plugin_e2e import ( "context" + "errors" + "fmt" "os" "os/exec" + "path/filepath" "strings" "testing" "time" @@ -14,34 +17,23 @@ import ( "github.com/tidwall/gjson" ) -// runIsolated runs bin like run() (harness.go), but with a per-call -// LARKSUITE_CLI_CONFIG_DIR (t.TempDir()) and LARKSUITE_CLI_REMOTE_META=off. -// This is needed ONLY by TestDegradeStubMetadataSchema below: without it, the -// fork inherits this developer machine's real ~/.lark-cli cache (a prior -// `lark-cli` invocation on this box already populated remote_meta.json) and/or -// makes a live network fetch, either of which would populate the runtime -// catalog with real data and hide the #1764 stub-metadata degrade path this -// test exists to pin. harness.go's run() has no env-override parameter and -// must not be modified to add one (out of scope for this task), so this is a -// small local variant of the same subprocess-capture logic. -func runIsolated(t *testing.T, bin string, args ...string) result { +// runEnv runs bin as a subprocess with the given full environment, capturing +// stdout/stderr/exit. It is the shared capture used by the isolated runners +// below; harness.go's run() (which inherits the host env) is left untouched. +func runEnv(t *testing.T, bin string, env []string, args ...string) result { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() c := exec.CommandContext(ctx, bin, args...) - c.Env = append(os.Environ(), - "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", - "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", - "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), - "LARKSUITE_CLI_REMOTE_META=off", - ) + c.Env = env var stdout, stderr strings.Builder c.Stdout = &stdout c.Stderr = &stderr err := c.Run() exit := 0 if err != nil { - if ee, ok := err.(*exec.ExitError); ok { + var ee *exec.ExitError + if errors.As(err, &ee) { exit = ee.ExitCode() } else { t.Fatalf("run %v: %v", args, err) @@ -50,6 +42,85 @@ func runIsolated(t *testing.T, bin string, args ...string) result { return result{stdout: stdout.String(), stderr: stderr.String(), exit: exit} } +// runIsolated runs bin with a per-call empty LARKSUITE_CLI_CONFIG_DIR and +// LARKSUITE_CLI_REMOTE_META=off, so the fork sees NO cached metadata and makes +// no network fetch, regardless of this developer machine's real ~/.lark-cli +// cache. Used to pin the cold-cache degrade path (TestDegradeStubMetadataSchema). +func runIsolated(t *testing.T, bin string, args ...string) result { + t.Helper() + env := append(os.Environ(), + "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", + "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", + "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), + "LARKSUITE_CLI_REMOTE_META=off", + ) + return runEnv(t, bin, env, args...) +} + +// seededCatalogVersion is far newer than the embedded stub's 0.0.0, so the +// runtime overlay in internal/registry unconditionally applies it. +const seededCatalogVersion = "9.9.9" + +// seededCatalogJSON is a remote_meta.json (registry.MergedRegistry) carrying one +// obviously-synthetic service. Seeding it into a bare-module fork's on-disk cache +// gives the runtime catalog real data WITHOUT any network, so a test can prove +// SchemaCatalog() consults that runtime catalog (issue #1764) rather than the +// embedded-only (empty stub) catalog. Fields mirror internal/meta.Service. +const seededCatalogJSON = `{ + "version": "9.9.9", + "services": [ + { + "name": "plugine2e", + "version": "v1", + "title": "plugin_e2e synthetic service", + "description": "synthetic fixture for the runtime-catalog test; not a real API", + "servicePath": "/open-apis/plugine2e/v1", + "resources": { + "widgets": { + "methods": { + "get": { + "id": "plugine2e.widgets.get", + "path": "/open-apis/plugine2e/v1/widgets/:id", + "httpMethod": "GET", + "description": "synthetic read method", + "risk": "read", + "accessTokens": ["tenant"], + "parameters": { + "id": {"type": "string", "location": "path", "required": true, "description": "synthetic id"} + } + } + } + } + } + } + ] +}` + +// runWithSeededCatalog runs bin against a fresh LARKSUITE_CLI_CONFIG_DIR whose +// cache already holds cacheJSON as remote_meta.json (plus a fresh, high-version +// cache-meta so the overlay applies and the TTL never triggers a refetch). Remote +// meta is left ON so the on-disk cache overlay is consulted, but a long +// LARKSUITE_CLI_META_TTL keeps the run offline and deterministic. This models a +// bare-module binary that has runtime metadata available from a warm cache. +func runWithSeededCatalog(t *testing.T, bin, cacheJSON string, args ...string) result { + t.Helper() + cfg := t.TempDir() + cacheDir := filepath.Join(cfg, "cache") + if err := os.MkdirAll(cacheDir, 0o755); err != nil { + t.Fatalf("mkdir cache dir: %v", err) + } + writeFile(t, filepath.Join(cacheDir, "remote_meta.json"), cacheJSON) + writeFile(t, filepath.Join(cacheDir, "remote_meta.meta.json"), + fmt.Sprintf(`{"last_check_at":%d,"version":%q,"brand":""}`, time.Now().Unix(), seededCatalogVersion)) + env := append(os.Environ(), + "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", + "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", + "LARKSUITE_CLI_CONFIG_DIR="+cfg, + "LARKSUITE_CLI_META_TTL=1000000", + ) + return runEnv(t, bin, env, args...) +} + // plainPlugin registers a minimal observer-only plugin with NO Restrict rule // -- unlike readonly_test.go's plugins, it cannot deny "schema" as // out-of-domain, so any failure the command produces below is the command's @@ -140,75 +211,34 @@ func TestDegradeStubMetadataSchema(t *testing.T) { } } -// transportAbortPlugin registers a transport.Provider whose Interceptor -// implements AbortableInterceptor and unconditionally rejects every request -// from PreRoundTripE, before the built-in RoundTripper chain (and therefore -// before any real network I/O) ever runs -- see extension/transport/types.go's -// AbortableInterceptor doc and internal/cmdutil/transport.go's RoundTrip. -const transportAbortPlugin = `// Code generated by plugin_e2e; DO NOT EDIT. -package plugin - -import ( - "context" - "errors" - "net/http" - - "github.com/larksuite/cli/extension/transport" -) - -type abortInterceptor struct{} - -func (abortInterceptor) PreRoundTrip(req *http.Request) func(*http.Response, error) { return nil } - -func (abortInterceptor) PreRoundTripE(req *http.Request) (func(*http.Response, error), error) { - return nil, errors.New("aborted for test") -} - -type abortProvider struct{} - -func (abortProvider) Name() string { return "abort-transport" } -func (abortProvider) ResolveInterceptor(ctx context.Context) transport.Interceptor { - return abortInterceptor{} -} - -func init() { - transport.Register(abortProvider{}) -} -` - -// TestSubsystemTransportAbort pins the transport.AbortableInterceptor offline -// effect: registering a Provider whose PreRoundTripE always errors turns -// every outbound API call into an abort before any network round trip. The -// destination URL embedded in the message varies run-to-run (it depends on -// whether the SDK fetches an OAuth token first or calls the target endpoint -// first), so the assertion is pinned to the stable part only. Observed real -// output for `docs +fetch --doc nonexistent` (a real read-risk network -// command, run twice across separate `go test -count` invocations): +// TestRuntimeCatalogResolvesSchema pins the PRIMARY #1764 fix: a bare-module fork +// (embedded stub only) resolves `schema` against the RUNTIME catalog seeded from +// the on-disk cache, not the embedded-only catalog. Before f0b6f35f the module +// build read the embedded-only catalog and returned "Unknown service: " even +// though the runtime registry had metadata; after it, registry.SchemaCatalog() +// falls back to the merged runtime catalog and the lookup succeeds. // -// run 1: exit=4 stderr={"ok":false,"identity":"user","error":{"type":"network", -// "subtype":"transport","message":"API call failed: Post -// \"https://open.feishu.cn/open-apis/authen/v2/oauth/token\": extension -// \"abort-transport\" aborted round trip: aborted for test"}} -// run 2: exit=4 stderr={... same shape, message targets the docs fetch -// endpoint instead of the OAuth token URL ...} -func TestSubsystemTransportAbort(t *testing.T) { - bin := buildFork(t, "transport-abort", transportAbortPlugin) - res := run(t, bin, "docs", "+fetch", "--doc", "nonexistent") +// This is the counterpart to TestDegradeStubMetadataSchema: that test pins the +// cold-cache corner (no runtime data -> graceful "No API metadata available"); +// this one pins the warm-cache main path (runtime data present -> schema works), +// so a regression that re-embeds the embedded-only lookup fails HERE with +// "Unknown service" rather than silently passing. +func TestRuntimeCatalogResolvesSchema(t *testing.T) { + bin := buildFork(t, "plain", plainPlugin) + res := runWithSeededCatalog(t, bin, seededCatalogJSON, "schema", "plugine2e") t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - if res.exit != 4 { - t.Fatalf("exit=%d want 4; stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - } - if !gjson.Valid(res.stderr) { - t.Fatalf("stderr not JSON: %s", res.stderr) + out := res.stdout + res.stderr + if strings.Contains(out, "Unknown service") { + t.Fatalf("schema returned \"Unknown service\" -> runtime catalog NOT consulted (issue #1764 regression); out=%s", out) } - if got := gjson.Get(res.stderr, "error.type").String(); got != "network" { - t.Errorf("error.type=%q want network", got) + if strings.Contains(out, "No API metadata available") { + t.Fatalf("schema saw no metadata -> the seeded runtime cache was not loaded; out=%s", out) } - if got := gjson.Get(res.stderr, "error.subtype").String(); got != "transport" { - t.Errorf("error.subtype=%q want transport", got) + if res.exit != 0 { + t.Fatalf("exit=%d want 0 (schema resolved from runtime catalog); stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) } - if msg := gjson.Get(res.stderr, "error.message").String(); !strings.Contains(msg, `extension "abort-transport" aborted round trip: aborted for test`) { - t.Errorf("error.message=%q want to contain the abort-transport reason", msg) + if !strings.Contains(out, "plugine2e") { + t.Errorf("schema output does not mention the seeded service; out=%s", out) } } diff --git a/tests/plugin_e2e/harness.go b/tests/plugin_e2e/harness.go index 1fa44d8dbb..d802cbdab7 100644 --- a/tests/plugin_e2e/harness.go +++ b/tests/plugin_e2e/harness.go @@ -21,6 +21,8 @@ package plugin_e2e import ( "context" + "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -48,17 +50,36 @@ func repoRoot() (string, error) { return strings.TrimSpace(string(out)), nil } -// gitArchive extracts HEAD's committed tree into dst. Only tracked files are -// included — gitignored build artifacts (e.g. the fetched meta_data.json) are -// absent, exactly as a module consumer would see them. +// gitArchive extracts HEAD's committed tree into dst by streaming `git archive` +// into `tar -x`. Only tracked files are included — gitignored build artifacts +// (e.g. the fetched meta_data.json) are absent, exactly as a module consumer +// would see them. It wires the two processes with an explicit pipe rather than a +// shell, so dst never reaches a shell command line. func gitArchive(root, dst string) error { - c := exec.Command("bash", "-c", "git archive HEAD | tar -x -C "+shellQuote(dst)) - c.Dir = root - return runCmd(c) + archive := exec.Command("git", "archive", "HEAD") + archive.Dir = root + extract := exec.Command("tar", "-x", "-C", dst) + pipe, err := archive.StdoutPipe() + if err != nil { + return err + } + extract.Stdin = pipe + var errBuf strings.Builder + archive.Stderr = &errBuf + extract.Stderr = &errBuf + if err := extract.Start(); err != nil { + return err + } + if err := archive.Run(); err != nil { + _ = extract.Wait() + return fmt.Errorf("git archive: %w: %s", err, errBuf.String()) + } + if err := extract.Wait(); err != nil { + return fmt.Errorf("tar extract: %w: %s", err, errBuf.String()) + } + return nil } -func shellQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } - // builtForks caches fork binaries by name so identical forks are built once. var builtForks = map[string]string{} @@ -144,7 +165,8 @@ func run(t *testing.T, bin string, args ...string) result { err := c.Run() exit := 0 if err != nil { - if ee, ok := err.(*exec.ExitError); ok { + var ee *exec.ExitError + if errors.As(err, &ee) { exit = ee.ExitCode() } else { t.Fatalf("run %v: %v", args, err) @@ -159,17 +181,3 @@ func writeFile(t *testing.T, path, content string) { t.Fatalf("write %s: %v", path, err) } } - -func runCmd(c *exec.Cmd) error { - if out, err := c.CombinedOutput(); err != nil { - return &cmdError{err: err, out: out} - } - return nil -} - -type cmdError struct { - err error - out []byte -} - -func (e *cmdError) Error() string { return e.err.Error() + ": " + string(e.out) } diff --git a/tests/plugin_e2e/restrict_test.go b/tests/plugin_e2e/restrict_test.go index 112c029f66..ff8ad74c9f 100644 --- a/tests/plugin_e2e/restrict_test.go +++ b/tests/plugin_e2e/restrict_test.go @@ -37,6 +37,14 @@ func init() { // structured field. func TestReadonlyDenial(t *testing.T) { bin := buildFork(t, "readonly", readonlyPlugin) + // Note: reason_code mixed_children_policy is intentionally NOT covered here. + // It requires a parent command whose *enumerated children* have mixed + // allow/deny outcomes, which needs the full command tree from API metadata. + // This L4 harness builds a bare-module fork (embedded stub only), so offline + // a parent like "sheets" has no known children and collapses to + // domain_not_allowed -- identical to the "leaf out of allow list" case and + // not a distinct reason_code. Covered instead by the in-process cmdpolicy + // unit tests, which construct a mixed-children tree directly. cases := []struct { name string args []string @@ -44,26 +52,10 @@ func TestReadonlyDenial(t *testing.T) { }{ {"write in allowed domain", []string{"docs", "+update", "--doc-token", "x", "--content", "y"}, "write_not_allowed"}, {"leaf out of allow list", []string{"schema"}, "domain_not_allowed"}, - {"parent group all children denied", []string{"sheets"}, "mixed_children_policy"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - res := run(t, bin, tc.args...) - if res.exit != 2 { - t.Fatalf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) - } - if !gjson.Valid(res.stderr) { - t.Fatalf("stderr not JSON: %s", res.stderr) - } - if got := gjson.Get(res.stderr, "error.type").String(); got != "validation" { - t.Errorf("error.type=%q want validation", got) - } - if got := gjson.Get(res.stderr, "error.subtype").String(); got != "failed_precondition" { - t.Errorf("error.subtype=%q want failed_precondition", got) - } - if hint := gjson.Get(res.stderr, "error.hint").String(); !strings.Contains(hint, "reason_code "+tc.reasonCode) { - t.Errorf("hint=%q want to contain reason_code %s", hint, tc.reasonCode) - } + assertReasonCodeEnvelope(t, run(t, bin, tc.args...), tc.reasonCode) }) } } From 85ccfb618a19bcfcc254420266f038398463f05f Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Thu, 9 Jul 2026 20:37:37 +0800 Subject: [PATCH 15/21] test: isolate plugin_e2e run() so all forks are deterministic offline The previous run() inherited the host environment, so a fork's startup metadata behavior depended on the developer machine's ~/.lark-cli cache and live network. Tests passed locally but flaked on CI (e.g. TestSubsystemCredentialBlock reaching a metadata fetch instead of the credential block). Move the isolated, offline environment (fresh empty LARKSUITE_CLI_CONFIG_DIR + LARKSUITE_CLI_REMOTE_META=off) into run() itself and fold the per-file runEnv/runIsolated helpers into the shared runWithEnv, so every test reproduces the bare-module customer state on any machine, including CI. Tests needing runtime metadata still seed it explicitly via runWithSeededCatalog. --- tests/plugin_e2e/degrade_subsystem_test.go | 51 ++-------------------- tests/plugin_e2e/harness.go | 35 ++++++++++++--- 2 files changed, 33 insertions(+), 53 deletions(-) diff --git a/tests/plugin_e2e/degrade_subsystem_test.go b/tests/plugin_e2e/degrade_subsystem_test.go index 40472bb736..7d972f345a 100644 --- a/tests/plugin_e2e/degrade_subsystem_test.go +++ b/tests/plugin_e2e/degrade_subsystem_test.go @@ -4,11 +4,8 @@ package plugin_e2e import ( - "context" - "errors" "fmt" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -17,46 +14,6 @@ import ( "github.com/tidwall/gjson" ) -// runEnv runs bin as a subprocess with the given full environment, capturing -// stdout/stderr/exit. It is the shared capture used by the isolated runners -// below; harness.go's run() (which inherits the host env) is left untouched. -func runEnv(t *testing.T, bin string, env []string, args ...string) result { - t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) - defer cancel() - c := exec.CommandContext(ctx, bin, args...) - c.Env = env - var stdout, stderr strings.Builder - c.Stdout = &stdout - c.Stderr = &stderr - err := c.Run() - exit := 0 - if err != nil { - var ee *exec.ExitError - if errors.As(err, &ee) { - exit = ee.ExitCode() - } else { - t.Fatalf("run %v: %v", args, err) - } - } - return result{stdout: stdout.String(), stderr: stderr.String(), exit: exit} -} - -// runIsolated runs bin with a per-call empty LARKSUITE_CLI_CONFIG_DIR and -// LARKSUITE_CLI_REMOTE_META=off, so the fork sees NO cached metadata and makes -// no network fetch, regardless of this developer machine's real ~/.lark-cli -// cache. Used to pin the cold-cache degrade path (TestDegradeStubMetadataSchema). -func runIsolated(t *testing.T, bin string, args ...string) result { - t.Helper() - env := append(os.Environ(), - "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", - "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", - "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), - "LARKSUITE_CLI_REMOTE_META=off", - ) - return runEnv(t, bin, env, args...) -} - // seededCatalogVersion is far newer than the embedded stub's 0.0.0, so the // runtime overlay in internal/registry unconditionally applies it. const seededCatalogVersion = "9.9.9" @@ -118,7 +75,7 @@ func runWithSeededCatalog(t *testing.T, bin, cacheJSON string, args ...string) r "LARKSUITE_CLI_CONFIG_DIR="+cfg, "LARKSUITE_CLI_META_TTL=1000000", ) - return runEnv(t, bin, env, args...) + return runWithEnv(t, bin, env, args...) } // plainPlugin registers a minimal observer-only plugin with NO Restrict rule @@ -147,8 +104,8 @@ func init() { // TestDegradeStubMetadataSchema pins the #1764 stub-metadata degrade path. // The clean tree embeds only the empty meta_data_default.json stub // (internal/registry/catalog.go's SchemaCatalog falls through to -// RuntimeCatalog when EmbeddedServicesTyped() is empty), and runIsolated -// additionally disables the remote overlay fetch and points the cache dir at +// RuntimeCatalog when EmbeddedServicesTyped() is empty), and run()'s isolated +// environment disables the remote overlay fetch and points the cache dir at // an empty tmp dir, so cmd/schema/schema.go's runSchema sees // catalog.Services() == 0 unconditionally -- the exact "offline with a cold // cache, remote meta off" branch documented at cmd/schema/schema.go:96-101. @@ -184,7 +141,7 @@ func TestDegradeStubMetadataSchema(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - res := runIsolated(t, bin, tc.args...) + res := run(t, bin, tc.args...) t.Logf("exit=%d stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) if res.exit != 2 { t.Fatalf("exit=%d want 2 (graceful validation exit); stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) diff --git a/tests/plugin_e2e/harness.go b/tests/plugin_e2e/harness.go index d802cbdab7..39f0fb4b3a 100644 --- a/tests/plugin_e2e/harness.go +++ b/tests/plugin_e2e/harness.go @@ -148,17 +148,40 @@ type result struct { exit int } -// run executes the fork binary with args and captures stdout/stderr/exit. -// Notifier env vars are suppressed to keep envelopes clean. +// run executes the fork binary with args in an isolated, offline environment and +// captures stdout/stderr/exit. Each call gets a fresh empty +// LARKSUITE_CLI_CONFIG_DIR and LARKSUITE_CLI_REMOTE_META=off, so the fork never +// inherits the host's ~/.lark-cli cache or makes a startup metadata fetch to the +// open platform. That reproduces the bare-module customer state (no embedded +// metadata, cold cache) deterministically on any machine, including CI: without +// it, whether a command's assertion is reached depends on whether a live network +// fetch happened to succeed. Tests that need runtime metadata seed it explicitly +// via runWithSeededCatalog. func run(t *testing.T, bin string, args ...string) result { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) - defer cancel() - c := exec.CommandContext(ctx, bin, args...) - c.Env = append(os.Environ(), + return runWithEnv(t, bin, isolatedEnv(t), args...) +} + +// isolatedEnv is the bare-module, offline environment shared by run() and (as a +// base) by runWithSeededCatalog. +func isolatedEnv(t *testing.T) []string { + t.Helper() + return append(os.Environ(), "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", + "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), + "LARKSUITE_CLI_REMOTE_META=off", ) +} + +// runWithEnv runs bin as a subprocess with the given full environment, capturing +// stdout/stderr/exit. +func runWithEnv(t *testing.T, bin string, env []string, args ...string) result { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + c := exec.CommandContext(ctx, bin, args...) + c.Env = env var stdout, stderr strings.Builder c.Stdout = &stdout c.Stderr = &stderr From 5b235130fbc9b79ba82576278c062b4659b751ba Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Thu, 9 Jul 2026 20:39:36 +0800 Subject: [PATCH 16/21] test: use separate stderr buffers in gitArchive to avoid data race archive.Stderr and extract.Stderr shared one strings.Builder; os/exec spawns a stderr-copy goroutine per command, so git archive and tar (which run concurrently) could write the builder simultaneously. strings.Builder is not concurrency-safe, so go test -race would flag it on any run where both processes emit stderr. Give each process its own buffer and report the relevant one on failure. Flagged by CodeRabbit on PR #1840. --- tests/plugin_e2e/harness.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/plugin_e2e/harness.go b/tests/plugin_e2e/harness.go index 39f0fb4b3a..703a5cbc53 100644 --- a/tests/plugin_e2e/harness.go +++ b/tests/plugin_e2e/harness.go @@ -64,18 +64,22 @@ func gitArchive(root, dst string) error { return err } extract.Stdin = pipe - var errBuf strings.Builder - archive.Stderr = &errBuf - extract.Stderr = &errBuf + // Each process gets its own stderr buffer: os/exec spawns a copy goroutine + // per command, so a shared strings.Builder would be written concurrently by + // both (git archive and tar run in parallel) -- a data race, since + // strings.Builder is not concurrency-safe. + var archiveErr, extractErr strings.Builder + archive.Stderr = &archiveErr + extract.Stderr = &extractErr if err := extract.Start(); err != nil { return err } if err := archive.Run(); err != nil { _ = extract.Wait() - return fmt.Errorf("git archive: %w: %s", err, errBuf.String()) + return fmt.Errorf("git archive: %w: %s", err, archiveErr.String()) } if err := extract.Wait(); err != nil { - return fmt.Errorf("tar extract: %w: %s", err, errBuf.String()) + return fmt.Errorf("tar extract: %w: %s", err, extractErr.String()) } return nil } From 2487355ddef149f54e5dd0a4d3e25d279279996a Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Fri, 10 Jul 2026 14:22:39 +0800 Subject: [PATCH 17/21] test: assert the docs request specifically and keep sidecar e2e offline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on the sidecar round-trip test surfaced two gaps: 1. False-green window: docs +fetch --as user routes TWO requests through the proxy — the mandatory /open-apis/authen/v1/user_info identity probe (enrichUserInfo verifies the sentinel UAT) and then the docs call. The sinks kept only the last request, so assertions could pass against the auxiliary probe. Capture every request per stub, track the HMAC verify outcome per request, and scope all assertions to the docs request (POST /open-apis/docs_ai/v1/documents//fetch), additionally checking method and X-Lark-Proxy-Target=open.feishu.cn. 2. Real network egress: runFork did not set LARKSUITE_CLI_REMOTE_META=off, so the fork's startup metadata refresh hit the real open.feishu.cn/api/tools/open/api_definition, contradicting the offline/secret-free contract. Set it; the run is now fully offline. runFork now also returns the fork's outcome and the test asserts exit 0 with an ok:true envelope, so the round trip must be a genuine end-to-end success rather than bytes that happened to flow. The mock upstream answers the identity probe with a parseable user_info body to keep the user resolution clean. --- tests/sidecar_e2e/roundtrip_test.go | 287 ++++++++++++++++++++++------ 1 file changed, 228 insertions(+), 59 deletions(-) diff --git a/tests/sidecar_e2e/roundtrip_test.go b/tests/sidecar_e2e/roundtrip_test.go index 6a002b3063..3fb79ab369 100644 --- a/tests/sidecar_e2e/roundtrip_test.go +++ b/tests/sidecar_e2e/roundtrip_test.go @@ -50,6 +50,8 @@ package sidecar_e2e import ( "bytes" "context" + "encoding/json" + "errors" "fmt" "io" "net/http" @@ -71,6 +73,28 @@ const ( testProxyKey = "test-proxy-key-not-a-real-secret-000000000000" testAppID = "cli_test_app_not_real" injectedToken = "fake-injected-token-not-real" + + // testDocToken is the --doc argument runFork passes; the docs +fetch call + // becomes POST /open-apis/docs_ai/v1/documents//fetch. Sharing + // it keeps the request marker below in sync with the command invocation. + testDocToken = "nonexistent" + + // docsReqMarker identifies the TARGET docs +fetch request among every + // request the fork routes through the proxy. `docs +fetch --as user` + // resolves a sentinel UAT, and the credential layer then verifies it with + // a mandatory /open-apis/authen/v1/user_info probe (see + // internal/credential/credential_provider.go enrichUserInfo) — so a second + // request also flows through the sidecar. Asserting on whichever arrived + // last would let that identity probe masquerade as the docs request; we + // filter for the docs call explicitly instead. + docsReqMarker = "/documents/" + testDocToken + "/fetch" + + // wantProxyTargetHost is the real Feishu open-platform host the interceptor + // must name as the proxy target for BRAND=feishu. The request is never + // actually forwarded there (the in-test sidecar redirects to the mock); the + // header only records where the fork BELIEVED it was going, and it is HMAC + // signing input, so it must be exactly the real host. + wantProxyTargetHost = "open.feishu.cn" ) // TestSidecarHMACRoundTrip drives the whole wire protocol as three named @@ -85,9 +109,27 @@ func TestSidecarHMACRoundTrip(t *testing.T) { // One real external process: lark-cli built with -tags authsidecar, run // fully offline against the in-test sidecar. bin := buildAuthsidecarFork(t) - runFork(t, bin, sc.URL) + res := runFork(t, bin, sc.URL) + + // Diagnostic dump (shown only on failure or -v): the full request set, so a + // failure makes plain which requests flowed and which one the assertions + // targeted, instead of guessing about the last-arriving request. + for _, s := range sc.seenAll() { + t.Logf("sidecar saw: %s %s target=%q identity=%q verifyRan=%v verifyErr=%v", + s.req.method, s.req.path, s.req.headers.Get(sidecar.HeaderProxyTarget), + s.req.headers.Get(sidecar.HeaderProxyIdentity), s.verifyRan, s.verifyErr) + } + for _, r := range upstream.sink.all() { + t.Logf("upstream saw: %s %s auth=%q", r.method, r.path, r.headers.Get("Authorization")) + } + t.Logf("fork exit=%d\nstdout=%s\nstderr=%s", res.exit, res.stdout, res.stderr) - // Assert the three properties of a correct round trip. + // Assert the fork's command itself succeeded end-to-end, not just that some + // bytes reached the sidecar. + assertForkSucceeded(t, res) + + // Assert the three properties of a correct round trip, scoped to the DOCS + // request (not an auxiliary identity probe). assertInterceptorSigned(t, sc) // (a)+(c) fork -> sidecar assertInjectedTokenReachedUpstream(t, upstream) // (b) sidecar -> upstream } @@ -104,29 +146,45 @@ type capturedRequest struct { body []byte } -// requestSink stores the request a stub server saw, guarded so the httptest -// handler goroutine and the test goroutine can hand it over safely. +// requestSink stores EVERY request a stub server saw, in arrival order, +// guarded so the httptest handler goroutine and the test goroutine can hand +// them over safely. Capturing all requests (not just the last) is what closes +// the false-green window: the fork may route more than one request through the +// proxy, and the target docs request is not guaranteed to be the last. type requestSink struct { - mu sync.Mutex - req *capturedRequest + mu sync.Mutex + reqs []*capturedRequest } -func (s *requestSink) capture(r *http.Request, body []byte) { - snap := capturedRequest{ +func (s *requestSink) capture(r *http.Request, body []byte) *capturedRequest { + snap := &capturedRequest{ method: r.Method, path: r.URL.RequestURI(), headers: r.Header.Clone(), body: body, } s.mu.Lock() - s.req = &snap + s.reqs = append(s.reqs, snap) s.mu.Unlock() + return snap +} + +func (s *requestSink) all() []*capturedRequest { + s.mu.Lock() + defer s.mu.Unlock() + return append([]*capturedRequest(nil), s.reqs...) } -func (s *requestSink) get() *capturedRequest { +// find returns the first captured request whose path contains sub, or nil. +func (s *requestSink) find(sub string) *capturedRequest { s.mu.Lock() defer s.mu.Unlock() - return s.req + for _, r := range s.reqs { + if strings.Contains(r.path, sub) { + return r + } + } + return nil } // --- mock upstream (stands in for open.feishu.cn) -------------------------- @@ -144,23 +202,44 @@ func startMockUpstream(t *testing.T) *mockUpstream { m.sink.capture(r, body) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"code":0,"msg":"success","data":{"document":{"content":"mock content"}}}`)) + // Respond per path so each forwarded request parses as success: the + // identity probe needs authen/v1/user_info's {data:{open_id,name}} to + // resolve cleanly; the docs +fetch is satisfied by the generic code:0 + // envelope. A single canned body would make the identity probe error. + _, _ = w.Write(mockResponseFor(r.URL.Path)) })) t.Cleanup(m.Close) return m } +// mockResponseFor returns a minimal success body matching the API the given +// path addresses. Unknown paths get a generic code:0 envelope. +func mockResponseFor(path string) []byte { + if strings.Contains(path, "/authen/v1/user_info") { + return []byte(`{"code":0,"msg":"success","data":{"open_id":"ou_mock","name":"mock user"}}`) + } + return []byte(`{"code":0,"msg":"success","data":{}}`) +} + // --- in-test sidecar (mirrors server-demo/handler.go verify+inject) -------- +// sidecarSeen is one request the in-test sidecar received, together with the +// per-request verification outcome. Tracking this per request (not as a single +// last-write-wins field) lets assertions check the verification that belongs +// to the DOCS request specifically. +type sidecarSeen struct { + req *capturedRequest + verifyRan bool + verifyErr error +} + type inTestSidecar struct { *httptest.Server key []byte upstreamURL string - sink requestSink - mu sync.Mutex // guards verifyRan/verifyErr - verifyRan bool - verifyErr error + mu sync.Mutex // guards seen + seen []sidecarSeen } func startInTestSidecar(t *testing.T, key []byte, upstreamURL string) *inTestSidecar { @@ -174,9 +253,17 @@ func startInTestSidecar(t *testing.T, key []byte, upstreamURL string) *inTestSid // handle is the request flow: capture -> verify (steps 0-4) -> inject+forward. func (s *inTestSidecar) handle(w http.ResponseWriter, r *http.Request) { body, _ := io.ReadAll(r.Body) - s.sink.capture(r, body) + snap := &capturedRequest{ + method: r.Method, + path: r.URL.RequestURI(), + headers: r.Header.Clone(), + body: body, + } - authHeader, ok := s.verifyProxyRequest(w, r, body) + authHeader, verifyRan, verifyErr, ok := s.verifyProxyRequest(w, r, body) + s.mu.Lock() + s.seen = append(s.seen, sidecarSeen{req: snap, verifyRan: verifyRan, verifyErr: verifyErr}) + s.mu.Unlock() if !ok { return } @@ -185,15 +272,15 @@ func (s *inTestSidecar) handle(w http.ResponseWriter, r *http.Request) { // verifyProxyRequest mirrors server-demo/handler.go steps 0-4: protocol // version, body SHA256, target validation, and HMAC signature verification. -// It records whether verification ran and its result (for assertions) and -// returns the auth header the client committed to. On any failure it writes -// the HTTP error and returns ok=false. -func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Request, body []byte) (authHeader string, ok bool) { +// It returns the auth header the client committed to, whether verification +// (step 4) actually ran, and its result. On any pre-step-4 failure it writes +// the HTTP error and returns ok=false with verifyRan=false. +func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Request, body []byte) (authHeader string, verifyRan bool, verifyErr error, ok bool) { // Step 0: protocol version. version := r.Header.Get(sidecar.HeaderProxyVersion) if version != sidecar.ProtocolV1 { http.Error(w, "unsupported "+sidecar.HeaderProxyVersion+": "+version, http.StatusBadRequest) - return "", false + return "", false, nil, false } // Step 1-2: timestamp + body SHA256. @@ -201,14 +288,16 @@ func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Reques claimedSHA := r.Header.Get(sidecar.HeaderBodySHA256) if claimedSHA == "" || claimedSHA != sidecar.BodySHA256(body) { http.Error(w, "body SHA256 mismatch", http.StatusBadRequest) - return "", false + return "", false, nil, false } // Step 3: target host, identity, auth-header (all covered by the sig). targetHost, perr := parseTargetHost(r.Header.Get(sidecar.HeaderProxyTarget)) if perr != nil { http.Error(w, "invalid "+sidecar.HeaderProxyTarget+": "+perr.Error(), http.StatusForbidden) - return "", false + // verifyRan=false: step 4 never ran; surface the parse error so the + // diagnostic dump shows why this request was rejected early. + return "", false, perr, false } identity := r.Header.Get(sidecar.HeaderProxyIdentity) authHeader = r.Header.Get(sidecar.HeaderProxyAuthHeader) @@ -224,15 +313,11 @@ func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Reques Identity: identity, AuthHeader: authHeader, }, r.Header.Get(sidecar.HeaderProxySignature)) - s.mu.Lock() - s.verifyRan = true - s.verifyErr = err - s.mu.Unlock() if err != nil { http.Error(w, "HMAC verification failed: "+err.Error(), http.StatusUnauthorized) - return "", false + return "", true, err, false } - return authHeader, true + return authHeader, true, nil, true } // forwardWithInjectedToken mirrors server-demo's inject+forward. Unlike @@ -281,11 +366,23 @@ func (s *inTestSidecar) forwardWithInjectedToken(w http.ResponseWriter, r *http. _, _ = w.Write(respBody) } -// verifyResult reports whether step 4 ran and, if so, its error. -func (s *inTestSidecar) verifyResult() (ran bool, err error) { +// seenAll returns a copy of every request the sidecar received, in order. +func (s *inTestSidecar) seenAll() []sidecarSeen { s.mu.Lock() defer s.mu.Unlock() - return s.verifyRan, s.verifyErr + return append([]sidecarSeen(nil), s.seen...) +} + +// findSeen returns the first received request whose path contains sub, or nil. +func (s *inTestSidecar) findSeen(sub string) *sidecarSeen { + s.mu.Lock() + defer s.mu.Unlock() + for i := range s.seen { + if strings.Contains(s.seen[i].req.path, sub) { + return &s.seen[i] + } + } + return nil } // isProxyHeader reports whether name is one of the sidecar wire-protocol @@ -352,10 +449,21 @@ func buildAuthsidecarFork(t *testing.T) string { return bin } -// runFork runs the fork against the in-test sidecar, fully offline. The fork's -// exit status is logged but NOT asserted — this test judges wire behavior -// (what reached the sidecar/upstream), not the command's own success. -func runFork(t *testing.T, binPath, sidecarURL string) { +// forkResult is the fork subprocess outcome. +type forkResult struct { + exit int + stdout string + stderr string +} + +// runFork runs the fork against the in-test sidecar, fully offline, and returns +// its exit code and captured output. LARKSUITE_CLI_REMOTE_META=off is essential: +// without it the fork's startup metadata refresh hits the real +// open.feishu.cn/api/tools/open/api_definition (internal/registry/remote.go), +// which both breaks the "offline, secret-free" contract and makes the run +// depend on live network. With it set, the command still completes and the +// docs request still flows through the sidecar, but nothing leaves the machine. +func runFork(t *testing.T, binPath, sidecarURL string) forkResult { t.Helper() scURL, err := url.Parse(sidecarURL) if err != nil { @@ -364,13 +472,14 @@ func runFork(t *testing.T, binPath, sidecarURL string) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() - cmd := exec.CommandContext(ctx, binPath, "docs", "+fetch", "--doc", "nonexistent", "--as", "user") + cmd := exec.CommandContext(ctx, binPath, "docs", "+fetch", "--doc", testDocToken, "--as", "user") cmd.Env = append(os.Environ(), "LARKSUITE_CLI_AUTH_PROXY=http://"+scURL.Host, "LARKSUITE_CLI_PROXY_KEY="+testProxyKey, "LARKSUITE_CLI_APP_ID="+testAppID, "LARKSUITE_CLI_BRAND=feishu", "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), + "LARKSUITE_CLI_REMOTE_META=off", "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", ) @@ -378,9 +487,16 @@ func runFork(t *testing.T, binPath, sidecarURL string) { cmd.Stdout = &stdout cmd.Stderr = &stderr runErr := cmd.Run() - t.Logf("fork exit error (informational only, not asserted): %v", runErr) - t.Logf("fork stdout: %s", stdout.String()) - t.Logf("fork stderr: %s", stderr.String()) + exit := 0 + if runErr != nil { + var ee *exec.ExitError + if errors.As(runErr, &ee) { + exit = ee.ExitCode() + } else { + t.Fatalf("run fork: %v", runErr) + } + } + return forkResult{exit: exit, stdout: stdout.String(), stderr: stderr.String()} } // repoRoot resolves the lark-cli module root from the test's working @@ -396,24 +512,59 @@ func repoRoot(t *testing.T) string { // --- assertions ------------------------------------------------------------ -// assertInterceptorSigned checks the fork -> sidecar hop (assertions a + c): -// the real interceptor ran (all proxy headers present, identity=user), stripped -// every real/sentinel auth header before signing, and produced a signature that +// assertForkSucceeded checks the fork command itself completed the round trip: +// exit 0 and an ok:true JSON envelope on stdout. This is what makes the docs +// request a genuine success path, not merely bytes that happened to flow. +func assertForkSucceeded(t *testing.T, res forkResult) { + t.Helper() + if res.exit != 0 { + t.Fatalf("fork exit=%d want 0; stdout=%s stderr=%s", res.exit, res.stdout, res.stderr) + } + // Parse rather than substring-match: the CLI pretty-prints stdout, so the + // envelope reads "ok": true (with a space), and the field's truth — not its + // serialized spelling — is what proves the round trip succeeded. + var env struct { + OK bool `json:"ok"` + } + if err := json.Unmarshal([]byte(res.stdout), &env); err != nil { + t.Fatalf("fork stdout is not a JSON envelope: %v; stdout=%s stderr=%s", err, res.stdout, res.stderr) + } + if !env.OK { + t.Fatalf("fork stdout ok != true (round trip did not succeed); stdout=%s stderr=%s", res.stdout, res.stderr) + } +} + +// assertInterceptorSigned checks the fork -> sidecar hop (assertions a + c) for +// the DOCS request specifically: the real interceptor ran (all proxy headers +// present, identity=user, method+path+target as expected), stripped every +// real/sentinel auth header before signing, and produced a signature that // verified against the shared key. func assertInterceptorSigned(t *testing.T, sc *inTestSidecar) { t.Helper() - got := sc.sink.get() - if got == nil { - t.Fatal("sidecar never received a request from the fork — interceptor did not route to AUTH_PROXY") + seen := sc.findSeen(docsReqMarker) + if seen == nil { + t.Fatalf("sidecar never received the docs request (marker %q) — interceptor did not route it to AUTH_PROXY; saw %v", + docsReqMarker, sidecarPaths(sc.seenAll())) + } + got := seen.req + if !seen.verifyRan { + t.Fatal("sidecar received the docs request but never reached HMAC verification (rejected earlier — see handler headers)") + } + if seen.verifyErr != nil { + t.Fatalf("HMAC verification failed on the fork's own signed docs request: %v", seen.verifyErr) } - ran, verifyErr := sc.verifyResult() - if !ran { - t.Fatal("sidecar received a request but never reached HMAC verification (rejected earlier — see handler headers)") + t.Logf("fork->sidecar docs headers: %v", got.headers) + + // Target/method/path: prove we asserted on the real docs call to the real + // Feishu open platform, not an auxiliary request. + if got.method != http.MethodPost { + t.Errorf("docs request method = %q, want POST", got.method) } - if verifyErr != nil { - t.Fatalf("HMAC verification failed on the fork's own signed request: %v", verifyErr) + if targetHost, err := parseTargetHost(got.headers.Get(sidecar.HeaderProxyTarget)); err != nil { + t.Errorf("docs request %s invalid: %v", sidecar.HeaderProxyTarget, err) + } else if targetHost != wantProxyTargetHost { + t.Errorf("docs request proxy target host = %q, want %q", targetHost, wantProxyTargetHost) } - t.Logf("fork->sidecar headers: %v", got.headers) // No real/sentinel auth ever left the fork: the interceptor strips the // sentinel before signing, so this hop must carry no auth header at all. @@ -443,15 +594,16 @@ func assertInterceptorSigned(t *testing.T, sc *inTestSidecar) { } // assertInjectedTokenReachedUpstream checks the sidecar -> upstream hop -// (assertion b): the mock saw exactly the sidecar-injected synthetic token, -// never a sentinel or a real one — proving injection actually happened. +// (assertion b) for the DOCS request: the mock saw exactly the sidecar-injected +// synthetic token, never a sentinel or a real one — proving injection happened. func assertInjectedTokenReachedUpstream(t *testing.T, up *mockUpstream) { t.Helper() - got := up.sink.get() + got := up.sink.find(docsReqMarker) if got == nil { - t.Fatal("mock upstream never received a forwarded request — sidecar did not forward after verification") + t.Fatalf("mock upstream never received the forwarded docs request (marker %q) — sidecar did not forward it after verification; saw %v", + docsReqMarker, requestPaths(up.sink.all())) } - t.Logf("sidecar->mock headers: %v", got.headers) + t.Logf("sidecar->mock docs headers: %v", got.headers) wantAuth := "Bearer " + injectedToken gotAuth := got.headers.Get("Authorization") @@ -464,3 +616,20 @@ func assertInjectedTokenReachedUpstream(t *testing.T, up *mockUpstream) { t.Fatalf("mock upstream received a sentinel token instead of the injected one: %q", gotAuth) } } + +// sidecarPaths / requestPaths render captured paths for failure messages. +func sidecarPaths(seen []sidecarSeen) []string { + paths := make([]string, len(seen)) + for i, s := range seen { + paths[i] = s.req.method + " " + s.req.path + } + return paths +} + +func requestPaths(reqs []*capturedRequest) []string { + paths := make([]string, len(reqs)) + for i, r := range reqs { + paths[i] = r.method + " " + r.path + } + return paths +} From 84e349eac69d65fe50023071c79e5ca5e1d2db93 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Fri, 10 Jul 2026 14:28:47 +0800 Subject: [PATCH 18/21] test: match docs request path exactly and assert proxy headers stay off upstream Two CodeRabbit findings on the previous commit: 1. The docs-request selectors used strings.Contains, so a wrong API prefix or version (e.g. docs_ai/v2) could still match while the mock answers any path with a generic success. Select by the full exact path /open-apis/docs_ai/v1/documents//fetch instead. 2. The upstream assertion proved token injection but would not notice the forward leaking sidecar wire-protocol headers. Assert every X-Lark-Proxy-* header and X-Lark-Body-Sha256 is absent from the forwarded docs request. --- tests/sidecar_e2e/roundtrip_test.go | 45 +++++++++++++++++++---------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/tests/sidecar_e2e/roundtrip_test.go b/tests/sidecar_e2e/roundtrip_test.go index 3fb79ab369..166cc4af69 100644 --- a/tests/sidecar_e2e/roundtrip_test.go +++ b/tests/sidecar_e2e/roundtrip_test.go @@ -79,15 +79,16 @@ const ( // it keeps the request marker below in sync with the command invocation. testDocToken = "nonexistent" - // docsReqMarker identifies the TARGET docs +fetch request among every - // request the fork routes through the proxy. `docs +fetch --as user` + // docsReqPath is the exact path of the TARGET docs +fetch request among + // every request the fork routes through the proxy. `docs +fetch --as user` // resolves a sentinel UAT, and the credential layer then verifies it with // a mandatory /open-apis/authen/v1/user_info probe (see // internal/credential/credential_provider.go enrichUserInfo) — so a second // request also flows through the sidecar. Asserting on whichever arrived // last would let that identity probe masquerade as the docs request; we - // filter for the docs call explicitly instead. - docsReqMarker = "/documents/" + testDocToken + "/fetch" + // select the docs call by its full path (exact match, not a substring — a + // wrong API prefix or version must not slip through). + docsReqPath = "/open-apis/docs_ai/v1/documents/" + testDocToken + "/fetch" // wantProxyTargetHost is the real Feishu open-platform host the interceptor // must name as the proxy target for BRAND=feishu. The request is never @@ -175,12 +176,12 @@ func (s *requestSink) all() []*capturedRequest { return append([]*capturedRequest(nil), s.reqs...) } -// find returns the first captured request whose path contains sub, or nil. -func (s *requestSink) find(sub string) *capturedRequest { +// find returns the first captured request whose path equals path, or nil. +func (s *requestSink) find(path string) *capturedRequest { s.mu.Lock() defer s.mu.Unlock() for _, r := range s.reqs { - if strings.Contains(r.path, sub) { + if r.path == path { return r } } @@ -373,12 +374,12 @@ func (s *inTestSidecar) seenAll() []sidecarSeen { return append([]sidecarSeen(nil), s.seen...) } -// findSeen returns the first received request whose path contains sub, or nil. -func (s *inTestSidecar) findSeen(sub string) *sidecarSeen { +// findSeen returns the first received request whose path equals path, or nil. +func (s *inTestSidecar) findSeen(path string) *sidecarSeen { s.mu.Lock() defer s.mu.Unlock() for i := range s.seen { - if strings.Contains(s.seen[i].req.path, sub) { + if s.seen[i].req.path == path { return &s.seen[i] } } @@ -541,10 +542,10 @@ func assertForkSucceeded(t *testing.T, res forkResult) { // verified against the shared key. func assertInterceptorSigned(t *testing.T, sc *inTestSidecar) { t.Helper() - seen := sc.findSeen(docsReqMarker) + seen := sc.findSeen(docsReqPath) if seen == nil { - t.Fatalf("sidecar never received the docs request (marker %q) — interceptor did not route it to AUTH_PROXY; saw %v", - docsReqMarker, sidecarPaths(sc.seenAll())) + t.Fatalf("sidecar never received the docs request (path %q) — interceptor did not route it to AUTH_PROXY; saw %v", + docsReqPath, sidecarPaths(sc.seenAll())) } got := seen.req if !seen.verifyRan { @@ -598,10 +599,10 @@ func assertInterceptorSigned(t *testing.T, sc *inTestSidecar) { // synthetic token, never a sentinel or a real one — proving injection happened. func assertInjectedTokenReachedUpstream(t *testing.T, up *mockUpstream) { t.Helper() - got := up.sink.find(docsReqMarker) + got := up.sink.find(docsReqPath) if got == nil { - t.Fatalf("mock upstream never received the forwarded docs request (marker %q) — sidecar did not forward it after verification; saw %v", - docsReqMarker, requestPaths(up.sink.all())) + t.Fatalf("mock upstream never received the forwarded docs request (path %q) — sidecar did not forward it after verification; saw %v", + docsReqPath, requestPaths(up.sink.all())) } t.Logf("sidecar->mock docs headers: %v", got.headers) @@ -615,6 +616,18 @@ func assertInjectedTokenReachedUpstream(t *testing.T, up *mockUpstream) { if gotAuth == "Bearer "+sidecar.SentinelUAT || gotAuth == "Bearer "+sidecar.SentinelTAT { t.Fatalf("mock upstream received a sentinel token instead of the injected one: %q", gotAuth) } + // The sidecar wire-protocol headers are between fork and sidecar only — + // the forward must strip every one of them. Token injection alone passing + // would still be a leak if signatures/timestamps/digests reached upstream. + for _, h := range []string{ + sidecar.HeaderProxyVersion, sidecar.HeaderProxyTarget, sidecar.HeaderProxyIdentity, + sidecar.HeaderProxySignature, sidecar.HeaderProxyTimestamp, sidecar.HeaderBodySHA256, + sidecar.HeaderProxyAuthHeader, + } { + if v := got.headers.Get(h); v != "" { + t.Errorf("proxy protocol header %s leaked to upstream (want stripped): %q", h, v) + } + } } // sidecarPaths / requestPaths render captured paths for failure messages. From fa8dc763a1e96c6831ade08c1f3346dde41e188b Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Tue, 14 Jul 2026 21:58:54 +0800 Subject: [PATCH 19/21] ci: exclude the whole tests/ subtree from coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage job's filter only excluded tests/cli_e2e, so the new tests/plugin_e2e (no build tag) survived go list ./... and ran a second time inside coverage — and since coverage sits in the results blocking loop, a plugin_e2e failure would have blocked merges, defeating the observe-only soft launch this PR establishes for plugin-integration. Exclude the whole tests/ subtree: everything under it is an L3/L4 suite with a dedicated job, the same rationale cli_e2e was excluded under. (sidecar_e2e is tag-gated and never entered go list; verified tests/plugin_e2e was the only survivor.) --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db457c4c45..5cbcc6f5be 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -202,7 +202,11 @@ jobs: run: python3 scripts/fetch_meta.py - name: Run tests with coverage run: | - packages=$(go list ./... | grep -v '^github.com/larksuite/cli/tests/cli_e2e$' | grep -v '^github.com/larksuite/cli/tests/cli_e2e/') + # tests/ holds only L3/L4 suites (cli_e2e, plugin_e2e, sidecar_e2e) that + # have dedicated jobs; exclude the whole subtree so none of them runs a + # second time here — and, crucially, so an observe-only suite's failure + # can never block merges through coverage's spot in the results loop. + packages=$(go list ./... | grep -v '^github.com/larksuite/cli/tests/') go test -race -coverprofile=coverage.txt -covermode=atomic $packages - name: Upload coverage to Codecov if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }} From 8b4929dd18e86b76a8ca84a6be4187252ce3a442 Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Tue, 14 Jul 2026 21:58:54 +0800 Subject: [PATCH 20/21] test: harden e2e harnesses per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - plugin_e2e/sidecar_e2e: strip the host's LARKSUITE_CLI_* namespace before composing fork environments. Appending overrides to a raw os.Environ() only isolated the variables we set; a developer machine exporting e.g. LARKSUITE_CLI_AUTH_PROXY leaked it into the fork, contradicting the determinism the harness comments claim. Verified by running both suites with hostile LARKSUITE_CLI_* exports. - plugin_e2e: guard builtForks with a mutex — the map was safe only under the unstated convention that no test uses t.Parallel(). - plugin_e2e: name the 60s timeout in run()'s failure message instead of surfacing a bare exit=-1. - sidecar_e2e: replace the overstated "mirrors steps 0-8 exactly" comment with a per-step coverage accounting (mirrored / replaced by assertion / not covered), renumber the in-test handler's step comments to match server-demo's, and mirror server-demo's step 1 timestamp presence check (server-demo enforces presence only; no freshness window exists there either). - Makefile: document that integration-test builds ~20 plugin_e2e forks (~1 min warm, GOPROXY downloads when cold). --- Makefile | 3 + tests/plugin_e2e/degrade_subsystem_test.go | 2 +- tests/plugin_e2e/harness.go | 40 ++++++++++- tests/sidecar_e2e/roundtrip_test.go | 80 +++++++++++++++++----- 4 files changed, 103 insertions(+), 22 deletions(-) diff --git a/Makefile b/Makefile index 59e6560bdc..a37b49e3b9 100644 --- a/Makefile +++ b/Makefile @@ -64,6 +64,9 @@ examples-build: go build ./extension/platform/examples/audit-observer go build ./extension/platform/examples/readonly-policy +# ./tests/... includes tests/plugin_e2e, which builds ~20 customer-fork +# binaries (~1 min warm; a cold module cache also downloads via GOPROXY). +# Deliberate: local `make test` exercises the L4 plugin contract by default. integration-test: build go test -v -count=1 ./tests/... diff --git a/tests/plugin_e2e/degrade_subsystem_test.go b/tests/plugin_e2e/degrade_subsystem_test.go index 7d972f345a..48a72d5528 100644 --- a/tests/plugin_e2e/degrade_subsystem_test.go +++ b/tests/plugin_e2e/degrade_subsystem_test.go @@ -69,7 +69,7 @@ func runWithSeededCatalog(t *testing.T, bin, cacheJSON string, args ...string) r writeFile(t, filepath.Join(cacheDir, "remote_meta.json"), cacheJSON) writeFile(t, filepath.Join(cacheDir, "remote_meta.meta.json"), fmt.Sprintf(`{"last_check_at":%d,"version":%q,"brand":""}`, time.Now().Unix(), seededCatalogVersion)) - env := append(os.Environ(), + env := append(baseEnv(), "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", "LARKSUITE_CLI_CONFIG_DIR="+cfg, diff --git a/tests/plugin_e2e/harness.go b/tests/plugin_e2e/harness.go index 703a5cbc53..d7f482deb9 100644 --- a/tests/plugin_e2e/harness.go +++ b/tests/plugin_e2e/harness.go @@ -27,6 +27,7 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "testing" "time" ) @@ -85,12 +86,22 @@ func gitArchive(root, dst string) error { } // builtForks caches fork binaries by name so identical forks are built once. -var builtForks = map[string]string{} +// builtForksMu guards it: no test in this package uses t.Parallel() today, but +// that is an implicit convention a future test could silently break, and an +// unguarded map write would then be a runtime panic. The lock is held across +// the whole build so concurrent callers also dedupe instead of racing to build +// the same fork twice. +var ( + builtForksMu sync.Mutex + builtForks = map[string]string{} +) // buildFork generates a customer module whose plugin package body is pluginSrc, // builds the fork, and returns the binary path. Forks are cached by name. func buildFork(t *testing.T, name, pluginSrc string) string { t.Helper() + builtForksMu.Lock() + defer builtForksMu.Unlock() if bin, ok := builtForks[name]; ok { return bin } @@ -166,11 +177,30 @@ func run(t *testing.T, bin string, args ...string) result { return runWithEnv(t, bin, isolatedEnv(t), args...) } +// baseEnv is the host environment with every LARKSUITE_CLI_* variable removed. +// Appending overrides to a raw os.Environ() only isolates the variables we +// explicitly set — a developer machine exporting, say, LARKSUITE_CLI_AUTH_PROXY +// or LARKSUITE_CLI_BRAND would leak them into the fork (the transport +// interceptor and credential providers read them via os.Getenv directly), +// breaking the "deterministic on any machine" guarantee. Stripping the whole +// namespace first makes the fork's CLI-facing environment exactly the +// variables the harness sets, everywhere. +func baseEnv() []string { + env := os.Environ() + kept := env[:0] + for _, kv := range env { + if !strings.HasPrefix(kv, "LARKSUITE_CLI_") { + kept = append(kept, kv) + } + } + return kept +} + // isolatedEnv is the bare-module, offline environment shared by run() and (as a // base) by runWithSeededCatalog. func isolatedEnv(t *testing.T) []string { t.Helper() - return append(os.Environ(), + return append(baseEnv(), "LARKSUITE_CLI_NO_UPDATE_NOTIFIER=1", "LARKSUITE_CLI_NO_SKILLS_NOTIFIER=1", "LARKSUITE_CLI_CONFIG_DIR="+t.TempDir(), @@ -190,6 +220,12 @@ func runWithEnv(t *testing.T, bin string, env []string, args ...string) result { c.Stdout = &stdout c.Stderr = &stderr err := c.Run() + // A fork that hangs is killed by the context and surfaces as a generic + // exit=-1 ExitError; name the timeout explicitly so the failure reads as + // "hung" rather than "crashed". + if ctx.Err() == context.DeadlineExceeded { + t.Fatalf("run %v: timed out after 60s; stdout=%s stderr=%s", args, stdout.String(), stderr.String()) + } exit := 0 if err != nil { var ee *exec.ExitError diff --git a/tests/sidecar_e2e/roundtrip_test.go b/tests/sidecar_e2e/roundtrip_test.go index 166cc4af69..4c2bcf6149 100644 --- a/tests/sidecar_e2e/roundtrip_test.go +++ b/tests/sidecar_e2e/roundtrip_test.go @@ -34,14 +34,33 @@ // same code paths this file would otherwise exercise via a real subprocess. // // So instead, this test builds its OWN in-test sidecar (an httptest.Server) -// that mirrors server-demo/handler.go's verify+inject steps 0-8 exactly, -// using the real protocol package (sidecar.Verify, sidecar.CanonicalRequest, +// built on the real protocol package (sidecar.Verify, sidecar.CanonicalRequest, // sidecar.BodySHA256, the Header* / Sentinel* / Identity* constants) — the -// same symbols server-demo itself uses. This is the standard shape for this -// kind of test: one real external process (the fork binary, compiled with -// the production interceptor code) plus two in-process httptest.Server -// stand-ins (sidecar, upstream). It proves the real wire protocol end-to-end -// without requiring live credentials, real feishu/lark hosts, or TLS. +// same symbols server-demo itself uses. Against server-demo/handler.go's +// numbered steps, the coverage accounting is: +// +// - steps 0-3 (protocol version, timestamp presence, body SHA256, HMAC +// verification): MIRRORED in the in-test handler. Note server-demo's +// step 1 checks timestamp presence only — no freshness/skew window +// exists there either; the timestamp's integrity is covered by the HMAC. +// - steps 4/5/5.5 (target-host / identity / auth-header allowlists): NOT +// enforced in the handler (a mock's 127.0.0.1 host can never be in a +// real allowlist); replaced by post-hoc test assertions that the docs +// request named the real Feishu host, identity=user, and the committed +// auth header was present. +// - step 6 (resolve real token): replaced by a synthetic injected token — +// the point of the offline design. +// - steps 7-10 (build forward request, inject, forward, relay response): +// mirrored in shape, except the forward goes to the in-test mock's URL +// instead of "https://"+targetHost (deliberate, documented on +// forwardWithInjectedToken). +// - step 11 (audit log): not covered; irrelevant to the wire contract. +// +// This is the standard shape for this kind of test: one real external +// process (the fork binary, compiled with the production interceptor code) +// plus two in-process httptest.Server stand-ins (sidecar, upstream). It +// proves the real wire protocol end-to-end without requiring live +// credentials, real feishu/lark hosts, or TLS. // // Every key/token/app-id here is an obviously-synthetic placeholder; nothing // in this file can authenticate against anything real. @@ -251,7 +270,7 @@ func startInTestSidecar(t *testing.T, key []byte, upstreamURL string) *inTestSid return s } -// handle is the request flow: capture -> verify (steps 0-4) -> inject+forward. +// handle is the request flow: capture -> verify (steps 0-3) -> inject+forward. func (s *inTestSidecar) handle(w http.ResponseWriter, r *http.Request) { body, _ := io.ReadAll(r.Body) snap := &capturedRequest{ @@ -271,11 +290,14 @@ func (s *inTestSidecar) handle(w http.ResponseWriter, r *http.Request) { s.forwardWithInjectedToken(w, r, body, authHeader) } -// verifyProxyRequest mirrors server-demo/handler.go steps 0-4: protocol -// version, body SHA256, target validation, and HMAC signature verification. -// It returns the auth header the client committed to, whether verification -// (step 4) actually ran, and its result. On any pre-step-4 failure it writes -// the HTTP error and returns ok=false with verifyRan=false. +// verifyProxyRequest mirrors server-demo/handler.go steps 0-3 (protocol +// version, timestamp presence, body SHA256, HMAC signature verification — +// including the target parse and identity/auth-header reads that feed the +// canonical signing string). The allowlist steps 4/5/5.5 are intentionally +// absent; the package comment's coverage accounting explains what replaces +// them. It returns the auth header the client committed to, whether HMAC +// verification (step 3) actually ran, and its result. On any earlier failure +// it writes the HTTP error and returns ok=false with verifyRan=false. func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Request, body []byte) (authHeader string, verifyRan bool, verifyErr error, ok bool) { // Step 0: protocol version. version := r.Header.Get(sidecar.HeaderProxyVersion) @@ -284,26 +306,34 @@ func (s *inTestSidecar) verifyProxyRequest(w http.ResponseWriter, r *http.Reques return "", false, nil, false } - // Step 1-2: timestamp + body SHA256. + // Step 1: timestamp presence (matching server-demo, which enforces + // presence only — the value's integrity is covered by the HMAC below; + // an empty-but-signed timestamp would otherwise verify fine). ts := r.Header.Get(sidecar.HeaderProxyTimestamp) + if ts == "" { + http.Error(w, "missing "+sidecar.HeaderProxyTimestamp, http.StatusBadRequest) + return "", false, nil, false + } + + // Step 2: body SHA256. claimedSHA := r.Header.Get(sidecar.HeaderBodySHA256) if claimedSHA == "" || claimedSHA != sidecar.BodySHA256(body) { http.Error(w, "body SHA256 mismatch", http.StatusBadRequest) return "", false, nil, false } - // Step 3: target host, identity, auth-header (all covered by the sig). + // Step 3 inputs: target host, identity, auth-header (all covered by the sig). targetHost, perr := parseTargetHost(r.Header.Get(sidecar.HeaderProxyTarget)) if perr != nil { http.Error(w, "invalid "+sidecar.HeaderProxyTarget+": "+perr.Error(), http.StatusForbidden) - // verifyRan=false: step 4 never ran; surface the parse error so the - // diagnostic dump shows why this request was rejected early. + // verifyRan=false: HMAC verification never ran; surface the parse error + // so the diagnostic dump shows why this request was rejected early. return "", false, perr, false } identity := r.Header.Get(sidecar.HeaderProxyIdentity) authHeader = r.Header.Get(sidecar.HeaderProxyAuthHeader) - // Step 4: verify HMAC signature over the canonical request. + // Step 3: verify HMAC signature over the canonical request. err := sidecar.Verify(s.key, sidecar.CanonicalRequest{ Version: version, Method: r.Method, @@ -474,7 +504,19 @@ func runFork(t *testing.T, binPath, sidecarURL string) forkResult { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() cmd := exec.CommandContext(ctx, binPath, "docs", "+fetch", "--doc", testDocToken, "--as", "user") - cmd.Env = append(os.Environ(), + // Strip the host's LARKSUITE_CLI_* namespace before appending overrides: a + // developer machine exporting, say, LARKSUITE_CLI_DEFAULT_AS or + // LARKSUITE_CLI_STRICT_MODE would otherwise leak into the fork (the sidecar + // credential provider reads them via os.Getenv), so the fork's CLI-facing + // environment is exactly the variables set below, on any machine. + env := os.Environ() + base := env[:0] + for _, kv := range env { + if !strings.HasPrefix(kv, "LARKSUITE_CLI_") { + base = append(base, kv) + } + } + cmd.Env = append(base, "LARKSUITE_CLI_AUTH_PROXY=http://"+scURL.Host, "LARKSUITE_CLI_PROXY_KEY="+testProxyKey, "LARKSUITE_CLI_APP_ID="+testAppID, From 064b3cb98880d2bb5f60b943185a2b201c9d3dbe Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Wed, 15 Jul 2026 10:18:43 +0800 Subject: [PATCH 21/21] ci: link observe-only soak graduation to tracking issue #1894 The results-job comment promised to add plugin-integration and sidecar-integration back into the blocking loop "once they have proven stable" with nothing holding that promise. Point it at issue #1894, which carries the graduation criteria (4 consecutive weeks with zero false positives). --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5cbcc6f5be..8fbaca2fe8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -476,8 +476,9 @@ jobs: # plugin-integration and sidecar-integration are intentionally NOT # in this loop yet: they run on every PR and their status is shown # in the table above, but a failure is observe-only (non-blocking) - # during the initial soak. Add them back here to make them required - # once they have proven stable. + # during the initial soak. Graduation to required is tracked in + # https://github.com/larksuite/cli/issues/1894 (criteria: 4 + # consecutive weeks with zero false positives). FAILED=0 for result in \ "${{ needs.fast-gate.result }}" \