From 9acd12125912c11699f23a976b223067b8c8bcb0 Mon Sep 17 00:00:00 2001 From: mazhe-nerd <106217973+mazhe-nerd@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:03:16 +0800 Subject: [PATCH 1/7] fix: update install message (#529) --- scripts/install-wizard.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/install-wizard.js b/scripts/install-wizard.js index 5691fdd05..b5b197d87 100644 --- a/scripts/install-wizard.js +++ b/scripts/install-wizard.js @@ -38,11 +38,11 @@ const messages = { step3Fail: "应用配置失败。运行以下命令重试: lark-cli config init --new", step4: "授权", step4NotFound: "未找到 lark-cli，跳过授权", - step4Confirm: "允许 AI 访问你的飞书数据（消息、文档、日历等）？", + step4Confirm: "是否允许 AI 访问你个人的消息、文档、日历等飞书 / Lark 数据，并以你的名义执行操作？", step4Skip: "跳过授权。后续运行 lark-cli auth login 完成授权", step4Done: "授权完成", step4Fail: "授权失败。运行以下命令重试: lark-cli auth login", - done: "安装完成！\n现在可以对你的 AI 工具（Claude Code、Trae 等）说：\"Feishu/Lark CLI 能帮我做什么？结合我的情况推荐一下从哪里开始\"", + done: "安装完成！\n可以和你的 AI 工具（如 Claude Code、Trae等）说：\"飞书/Lark CLI 能帮我做什么？结合我的情况推荐一下从哪里开始\"", cancelled: "安装已取消", }, en: { @@ -66,7 +66,7 @@ const messages = { step3Fail: "Failed to configure app. Run manually: lark-cli config init --new", step4: "Authorization", step4NotFound: "lark-cli not found. Skipping authorization", - step4Confirm: "Allow AI to access your Feishu/Lark data (messages, docs, calendar, etc.)?", + step4Confirm: "Allow the AI to access your messages, documents, calendar, and more in Feishu/Lark, and perform actions on your behalf?", step4Skip: "Skipped. Run lark-cli auth login to authorize later", step4Done: "Authorization complete", step4Fail: "Failed to authorize. Run lark-cli auth login to retry", From cd666422ace4712be3d4da12f1d40fd9db5bec96 Mon Sep 17 00:00:00 2001 From: kongenpei Date: Mon, 20 Apr 2026 19:14:45 +0800 Subject: [PATCH 2/7] fix(base): preserve attachment metadata on base uploads (#563) * fix: preserve attachment metadata on base uploads * test: cover attachment mime detection * fix: address attachment upload review feedback * fix: preserve source extension for attachment mime detection * fix: avoid registry test refresh data race * Revert "fix: avoid registry test refresh data race" This reverts commit c1d12d0cf1af9e0733f81158e7370b1a85ae52be. --------- Co-authored-by: kongenpei --- shortcuts/base/base_dryrun_ops_test.go | 2 + shortcuts/base/base_execute_test.go | 6 +- shortcuts/base/record_upload_attachment.go | 83 ++++++++++- .../base/record_upload_attachment_test.go | 136 ++++++++++++++++++ 4 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 shortcuts/base/record_upload_attachment_test.go diff --git a/shortcuts/base/base_dryrun_ops_test.go b/shortcuts/base/base_dryrun_ops_test.go index 64d53bdfd..22b0e07ea 100644 --- a/shortcuts/base/base_dryrun_ops_test.go +++ b/shortcuts/base/base_dryrun_ops_test.go @@ -137,6 +137,8 @@ func TestDryRunRecordOps(t *testing.T) { "bitable_file", "PATCH /open-apis/base/v3/bases/app_x/tables/tbl_1/records/rec_1", "report-final.pdf", + `"mime_type":"\u003cdetected_mime_type\u003e"`, + `"size":"\u003cfile_size\u003e"`, "deprecated_set_attachment", ) } diff --git a/shortcuts/base/base_execute_test.go b/shortcuts/base/base_execute_test.go index 35b8b1e13..e9f432ba2 100644 --- a/shortcuts/base/base_execute_test.go +++ b/shortcuts/base/base_execute_test.go @@ -1219,7 +1219,9 @@ func TestBaseRecordExecuteReadCreateDelete(t *testing.T) { !strings.Contains(updateBody, `"image_height":480`) || !strings.Contains(updateBody, `"deprecated_set_attachment":true`) || !strings.Contains(updateBody, `"file_token":"file_tok_1"`) || - !strings.Contains(updateBody, `"name":"report.txt"`) { + !strings.Contains(updateBody, `"name":"report.txt"`) || + !strings.Contains(updateBody, `"size":16`) || + !strings.Contains(updateBody, `"mime_type":"text/plain"`) { t.Fatalf("update body=%s", updateBody) } }) @@ -1370,6 +1372,8 @@ func TestBaseRecordExecuteReadCreateDelete(t *testing.T) { if !strings.Contains(updateBody, `"附件"`) || !strings.Contains(updateBody, `"file_token":"file_tok_big"`) || !strings.Contains(updateBody, `"name":"large-report.bin"`) || + !strings.Contains(updateBody, `"size":20971521`) || + !strings.Contains(updateBody, `"mime_type":"application/octet-stream"`) || !strings.Contains(updateBody, `"deprecated_set_attachment":true`) { t.Fatalf("update body=%s", updateBody) } diff --git a/shortcuts/base/record_upload_attachment.go b/shortcuts/base/record_upload_attachment.go index de0615d1f..67fde80f8 100644 --- a/shortcuts/base/record_upload_attachment.go +++ b/shortcuts/base/record_upload_attachment.go @@ -4,11 +4,15 @@ package base import ( + "bytes" "context" "errors" "fmt" + "io" + "mime" "path/filepath" "strings" + "unicode/utf8" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/output" @@ -105,6 +109,8 @@ func dryRunRecordUploadAttachment(_ context.Context, runtime *common.RuntimeCont map[string]interface{}{ "file_token": "", "name": fileName, + "mime_type": "", + "size": "", "deprecated_set_attachment": true, }, }, @@ -243,10 +249,14 @@ func normalizeAttachmentForPatch(attachment map[string]interface{}) map[string]i } func uploadAttachmentToBase(runtime *common.RuntimeContext, filePath, fileName, baseToken string, fileSize int64) (map[string]interface{}, error) { + mimeType, err := detectAttachmentMIMEType(runtime.FileIO(), filePath, fileName) + if err != nil { + return nil, err + } + parentNode := baseToken var ( fileToken string - err error ) if fileSize <= common.MaxDriveMediaUploadSinglePartSize { fileToken, err = common.UploadDriveMediaAll(runtime, common.DriveMediaUploadAllConfig{ @@ -272,7 +282,78 @@ func uploadAttachmentToBase(runtime *common.RuntimeContext, filePath, fileName, attachment := map[string]interface{}{ "file_token": fileToken, "name": fileName, + "mime_type": mimeType, + "size": fileSize, "deprecated_set_attachment": true, } return attachment, nil } + +func detectAttachmentMIMEType(fio fileio.FileIO, filePath, fileName string) (string, error) { + if byExt := strings.TrimSpace(mime.TypeByExtension(strings.ToLower(filepath.Ext(fileName)))); byExt != "" { + return stripMIMEParams(byExt), nil + } + if byExt := strings.TrimSpace(mime.TypeByExtension(strings.ToLower(filepath.Ext(filePath)))); byExt != "" { + return stripMIMEParams(byExt), nil + } + + f, err := fio.Open(filePath) + if err != nil { + return "", common.WrapInputStatError(err) + } + defer f.Close() + + buf := make([]byte, 512) + n, readErr := f.Read(buf) + if readErr != nil && !errors.Is(readErr, io.EOF) { + return "", output.ErrValidation("cannot read file: %s", readErr) + } + return detectAttachmentMIMEFromContent(buf[:n]), nil +} + +func stripMIMEParams(value string) string { + if i := strings.IndexByte(value, ';'); i != -1 { + value = value[:i] + } + return strings.TrimSpace(value) +} + +func detectAttachmentMIMEFromContent(content []byte) string { + if len(content) == 0 { + return "application/octet-stream" + } + if bytes.HasPrefix(content, []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'}) { + return "image/png" + } + if bytes.HasPrefix(content, []byte{0xff, 0xd8, 0xff}) { + return "image/jpeg" + } + if bytes.HasPrefix(content, []byte("GIF87a")) || bytes.HasPrefix(content, []byte("GIF89a")) { + return "image/gif" + } + if len(content) >= 12 && bytes.Equal(content[:4], []byte("RIFF")) && bytes.Equal(content[8:12], []byte("WEBP")) { + return "image/webp" + } + if bytes.HasPrefix(content, []byte("%PDF-")) { + return "application/pdf" + } + if looksLikeText(content) { + return "text/plain" + } + return "application/octet-stream" +} + +func looksLikeText(content []byte) bool { + if !utf8.Valid(content) { + return false + } + for _, r := range string(content) { + if r == '\n' || r == '\r' || r == '\t' { + continue + } + if r < 0x20 || r == 0x7f { + return false + } + } + return true +} diff --git a/shortcuts/base/record_upload_attachment_test.go b/shortcuts/base/record_upload_attachment_test.go new file mode 100644 index 000000000..69ff360e2 --- /dev/null +++ b/shortcuts/base/record_upload_attachment_test.go @@ -0,0 +1,136 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package base + +import ( + "bytes" + "io" + "io/fs" + "os" + "strings" + "testing" + + "github.com/larksuite/cli/extension/fileio" +) + +type attachmentTestFileIO struct { + openFile fileio.File + openErr error +} + +func (f attachmentTestFileIO) Open(string) (fileio.File, error) { return f.openFile, f.openErr } +func (attachmentTestFileIO) Stat(string) (fileio.FileInfo, error) { + return attachmentTestFileInfo{}, nil +} +func (attachmentTestFileIO) ResolvePath(path string) (string, error) { return path, nil } +func (attachmentTestFileIO) Save(string, fileio.SaveOptions, io.Reader) (fileio.SaveResult, error) { + return nil, nil +} + +type attachmentTestFileInfo struct{} + +func (attachmentTestFileInfo) Size() int64 { return 0 } +func (attachmentTestFileInfo) IsDir() bool { return false } +func (attachmentTestFileInfo) Mode() fs.FileMode { return 0 } + +type attachmentTestFile struct { + *bytes.Reader +} + +func newAttachmentTestFile(content []byte) attachmentTestFile { + return attachmentTestFile{Reader: bytes.NewReader(content)} +} + +func (attachmentTestFile) Close() error { return nil } + +type attachmentReadErrorFile struct{} + +func (attachmentReadErrorFile) Read([]byte) (int, error) { return 0, os.ErrPermission } +func (attachmentReadErrorFile) ReadAt([]byte, int64) (int, error) { return 0, io.EOF } +func (attachmentReadErrorFile) Close() error { return nil } + +func TestDetectAttachmentMIMETypeUsesExtension(t *testing.T) { + got, err := detectAttachmentMIMEType(nil, "ignored", "note.TXT") + if err != nil { + t.Fatalf("detectAttachmentMIMEType() error = %v", err) + } + if got != "text/plain" { + t.Fatalf("detectAttachmentMIMEType() = %q, want %q", got, "text/plain") + } +} + +func TestDetectAttachmentMIMETypeFallsBackToSourcePathExtension(t *testing.T) { + got, err := detectAttachmentMIMEType(nil, "report.docx", "report") + if err != nil { + t.Fatalf("detectAttachmentMIMEType() error = %v", err) + } + if got != "application/vnd.openxmlformats-officedocument.wordprocessingml.document" { + t.Fatalf("detectAttachmentMIMEType() = %q, want docx MIME type", got) + } +} + +func TestDetectAttachmentMIMETypeFallsBackToContent(t *testing.T) { + fio := attachmentTestFileIO{openFile: newAttachmentTestFile([]byte("hello from base attachment"))} + + got, err := detectAttachmentMIMEType(fio, "note", "note") + if err != nil { + t.Fatalf("detectAttachmentMIMEType() error = %v", err) + } + if got != "text/plain" { + t.Fatalf("detectAttachmentMIMEType() = %q, want %q", got, "text/plain") + } +} + +func TestDetectAttachmentMIMETypeWrapsOpenError(t *testing.T) { + fio := attachmentTestFileIO{openErr: os.ErrNotExist} + + _, err := detectAttachmentMIMEType(fio, "missing", "missing") + if err == nil { + t.Fatal("expected error for open failure") + } + if !strings.Contains(err.Error(), "cannot read file") { + t.Fatalf("error = %v, want wrapped read failure", err) + } +} + +func TestDetectAttachmentMIMETypeReturnsReadError(t *testing.T) { + fio := attachmentTestFileIO{openFile: attachmentReadErrorFile{}} + + _, err := detectAttachmentMIMEType(fio, "broken", "broken") + if err == nil { + t.Fatal("expected error for read failure") + } + if !strings.Contains(err.Error(), "cannot read file") { + t.Fatalf("error = %v, want read failure", err) + } +} + +func TestDetectAttachmentMIMEFromContent(t *testing.T) { + tests := []struct { + name string + content []byte + want string + }{ + {name: "empty", content: nil, want: "application/octet-stream"}, + {name: "png", content: []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'}, want: "image/png"}, + {name: "jpeg", content: []byte{0xff, 0xd8, 0xff, 0xe0}, want: "image/jpeg"}, + {name: "gif87a", content: []byte("GIF87a"), want: "image/gif"}, + {name: "gif89a", content: []byte("GIF89a"), want: "image/gif"}, + {name: "webp", content: []byte("RIFF1234WEBP"), want: "image/webp"}, + {name: "pdf", content: []byte("%PDF-1.7"), want: "application/pdf"}, + {name: "text", content: []byte("hello from base attachment"), want: "text/plain"}, + {name: "text with newline", content: []byte("hello\nworld\tok"), want: "text/plain"}, + {name: "control bytes", content: []byte{'h', 'i', 0x00}, want: "application/octet-stream"}, + {name: "binary fallback", content: []byte{0x00, 0x01, 0x02, 0x03}, want: "application/octet-stream"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := detectAttachmentMIMEFromContent(tt.content) + if got != tt.want { + t.Fatalf("detectAttachmentMIMEFromContent() = %q, want %q", got, tt.want) + } + }) + } +} From 5943a20e2bc6f97160aad23f4d973c874467f8f1 Mon Sep 17 00:00:00 2001 From: sang-neo03 Date: Mon, 20 Apr 2026 20:24:51 +0800 Subject: [PATCH 3/7] Feat/auth sidecar proxy (#532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(sidecar): add sidecar proxy for sandbox credential isolation Keep real secrets (app_secret, access_token) out of sandbox environments. CLI instances inside sandboxes connect to a trusted sidecar process via HTTP; the sidecar verifies HMAC-signed requests and injects real tokens before forwarding to the Lark API. Key components: - `auth proxy` subcommand to start the sidecar server (build tag: authsidecar) - Noop credential provider returns sentinel tokens in sidecar mode - Transport interceptor rewrites requests to sidecar with HMAC signature - Env provider yields to sidecar provider when AUTH_PROXY is set - Supports both feishu and lark brand endpoints * feat(sidecar): implement priority ordering for credential providers * feat(sidecar): strip client-supplied auth headers and improve shutdown logging * feat(sidecar): buffer request body to prevent HMAC mismatches on read errors * feat(sidecar): fix CI * refactor(sidecar): publish protocol package and move server to reference demo The sidecar server is no longer shipped as a `lark-cli auth proxy` subcommand. Instead, the CLI provides only the standard sidecar *client* (via `-tags authsidecar`), while the wire-protocol utilities are exposed as a public package for integrators to implement their own server. Changes: - Move `internal/sidecar/` → `sidecar/` so external integrators can import HMAC signing, headers, sentinels and address validators. - Remove `cmd/auth/proxy.go`, `proxy_stub.go`, `proxy_test.go` and the conditional registration in `cmd/auth/auth.go`. - Add `sidecar/server-demo/` — a reference server implementation behind the `authsidecar_demo` build tag. It reuses the lark-cli credential pipeline for local development; production integrators are expected to replace the credential layer with their own secrets source. - Update all internal imports from `internal/sidecar` to `sidecar`. Rationale: - Each integrator has different secrets management / HA / multi-tenant requirements, so a one-size-fits-all server doesn't belong in the shipped CLI. - Keeping the client in-tree guarantees all sandbox-side code stays protocol-compatible without a second repo to sync. - The public `sidecar/` package pins the wire protocol as a stable contract third-party servers must conform to. Build matrix after this change: - `go build` → standard CLI, no sidecar code - `go build -tags authsidecar` → CLI + sidecar client - `go build -tags authsidecar_demo \ ./sidecar/server-demo/` → reference server binary No production users are affected today because the server was not yet released; existing sidecar-client users are unchanged. * feat(sidecar): close 5 pre-release security gaps - Server: enforce https-only target (no path/query/userinfo), pin forwardURL to https:// — blocks cleartext token leak - Protocol v1: canonical now covers version/identity/auth-header, blocks identity-flip replay within drift window - Client: ValidateProxyAddr requires loopback or same-host alias, rejects userinfo and https (interceptor is http-only); cross-machine is out of scope - Build: non-authsidecar builds exit(2) when AUTH_PROXY is set, preventing silent fallback to env credentials - Demo: whitelist auth-header to Authorization / X-Lark-MCP-{UAT,TAT}, blocks token injection into Cookie / UA / X-Forwarded-For exfil paths --- .gitignore | 2 + extension/credential/registry.go | 23 +- extension/credential/registry_test.go | 26 + extension/credential/sidecar/provider.go | 131 ++++ extension/credential/sidecar/provider_test.go | 188 +++++ extension/transport/sidecar/interceptor.go | 178 +++++ .../transport/sidecar/interceptor_test.go | 265 +++++++ internal/envvars/envvars.go | 4 + main_authsidecar.go | 11 + main_noauthsidecar.go | 54 ++ main_noauthsidecar_test.go | 52 ++ sidecar/hmac.go | 88 +++ sidecar/hmac_test.go | 300 ++++++++ sidecar/protocol.go | 198 ++++++ sidecar/server-demo/README.md | 197 +++++ sidecar/server-demo/allowlist.go | 44 ++ sidecar/server-demo/audit.go | 51 ++ sidecar/server-demo/forward.go | 51 ++ sidecar/server-demo/handler.go | 271 +++++++ sidecar/server-demo/handler_test.go | 670 ++++++++++++++++++ sidecar/server-demo/main.go | 167 +++++ 21 files changed, 2969 insertions(+), 2 deletions(-) create mode 100644 extension/credential/sidecar/provider.go create mode 100644 extension/credential/sidecar/provider_test.go create mode 100644 extension/transport/sidecar/interceptor.go create mode 100644 extension/transport/sidecar/interceptor_test.go create mode 100644 main_authsidecar.go create mode 100644 main_noauthsidecar.go create mode 100644 main_noauthsidecar_test.go create mode 100644 sidecar/hmac.go create mode 100644 sidecar/hmac_test.go create mode 100644 sidecar/protocol.go create mode 100644 sidecar/server-demo/README.md create mode 100644 sidecar/server-demo/allowlist.go create mode 100644 sidecar/server-demo/audit.go create mode 100644 sidecar/server-demo/forward.go create mode 100644 sidecar/server-demo/handler.go create mode 100644 sidecar/server-demo/handler_test.go create mode 100644 sidecar/server-demo/main.go diff --git a/.gitignore b/.gitignore index ff1490812..822e8f314 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,5 @@ tests/mail/reports/ internal/registry/meta_data.json cmd/api/download.bin app.log +/sidecar-server-demo +/server-demo diff --git a/extension/credential/registry.go b/extension/credential/registry.go index 52ec9ebf8..fc71100fb 100644 --- a/extension/credential/registry.go +++ b/extension/credential/registry.go @@ -3,7 +3,10 @@ package credential -import "sync" +import ( + "sort" + "sync" +) var ( mu sync.Mutex @@ -11,12 +14,28 @@ var ( ) // Register registers a credential Provider. -// Providers are consulted in registration order. +// Providers are consulted in priority order (lowest value first). +// Providers that implement Priority() int are sorted accordingly; +// those that do not default to priority 10. // Typically called from init() via blank import. func Register(p Provider) { mu.Lock() defer mu.Unlock() providers = append(providers, p) + sort.SliceStable(providers, func(i, j int) bool { + return providerPriority(providers[i]) < providerPriority(providers[j]) + }) +} + +// providerPriority returns the priority of a provider. +// If the provider implements interface{ Priority() int }, that value is used; +// otherwise 10 is returned as the default priority. +// Lower values are consulted first. +func providerPriority(p Provider) int { + if pp, ok := p.(interface{ Priority() int }); ok { + return pp.Priority() + } + return 10 } // Providers returns all registered providers (snapshot). diff --git a/extension/credential/registry_test.go b/extension/credential/registry_test.go index e4ab0885b..0e4bf455f 100644 --- a/extension/credential/registry_test.go +++ b/extension/credential/registry_test.go @@ -37,6 +37,32 @@ func TestRegisterAndProviders(t *testing.T) { } } +type priorityProvider struct { + stubProvider + priority int +} + +func (p *priorityProvider) Priority() int { return p.priority } + +func TestRegister_PriorityOrder(t *testing.T) { + mu.Lock() + old := providers + providers = nil + mu.Unlock() + defer func() { mu.Lock(); providers = old; mu.Unlock() }() + + Register(&stubProvider{name: "env"}) // priority 10 (default) + Register(&priorityProvider{stubProvider: stubProvider{name: "sidecar"}, priority: 0}) // priority 0 (first) + + got := Providers() + if len(got) != 2 { + t.Fatalf("expected 2, got %d", len(got)) + } + if got[0].Name() != "sidecar" || got[1].Name() != "env" { + t.Errorf("expected sidecar before env, got %s, %s", got[0].Name(), got[1].Name()) + } +} + func TestProviders_ReturnsSnapshot(t *testing.T) { mu.Lock() old := providers diff --git a/extension/credential/sidecar/provider.go b/extension/credential/sidecar/provider.go new file mode 100644 index 000000000..99948939a --- /dev/null +++ b/extension/credential/sidecar/provider.go @@ -0,0 +1,131 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +// Package sidecar provides a noop credential provider for the auth sidecar +// proxy mode. When LARKSUITE_CLI_AUTH_PROXY is set, this provider supplies +// placeholder credentials so the CLI's auth pipeline can proceed normally. +// Real tokens are never present in the sandbox; the sidecar transport +// interceptor routes requests to the trusted sidecar process instead. +package sidecar + +import ( + "context" + "fmt" + "os" + + "github.com/larksuite/cli/extension/credential" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/sidecar" +) + +// Provider is the noop credential provider for sidecar mode. +type Provider struct{} + +func (p *Provider) Name() string { return "sidecar" } +func (p *Provider) Priority() int { return 0 } + +// ResolveAccount returns a minimal Account when sidecar mode is active. +// The account contains AppID and Brand from environment variables, a +// placeholder secret, and SupportedIdentities derived from STRICT_MODE. +// Returns nil, nil when sidecar mode is not active (AUTH_PROXY not set). +func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, error) { + proxyAddr := os.Getenv(envvars.CliAuthProxy) + if proxyAddr == "" { + return nil, nil // not in sidecar mode, skip + } + + if err := sidecar.ValidateProxyAddr(proxyAddr); err != nil { + return nil, &credential.BlockError{ + Provider: "sidecar", + Reason: fmt.Sprintf("invalid %s %q: %v", envvars.CliAuthProxy, proxyAddr, err), + } + } + + appID := os.Getenv(envvars.CliAppID) + if appID == "" { + return nil, &credential.BlockError{ + Provider: "sidecar", + Reason: envvars.CliAuthProxy + " is set but " + envvars.CliAppID + " is missing", + } + } + + if os.Getenv(envvars.CliProxyKey) == "" { + return nil, &credential.BlockError{ + Provider: "sidecar", + Reason: envvars.CliAuthProxy + " is set but " + envvars.CliProxyKey + " is missing", + } + } + + brand := credential.Brand(os.Getenv(envvars.CliBrand)) + if brand == "" { + brand = credential.BrandFeishu + } + + acct := &credential.Account{ + AppID: appID, + AppSecret: credential.NoAppSecret, + Brand: brand, + } + + // Parse DefaultAs + switch id := credential.Identity(os.Getenv(envvars.CliDefaultAs)); id { + case "", credential.IdentityAuto: + acct.DefaultAs = id + case credential.IdentityUser, credential.IdentityBot: + acct.DefaultAs = id + default: + return nil, &credential.BlockError{ + Provider: "sidecar", + Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envvars.CliDefaultAs, id), + } + } + + // Parse SupportedIdentities from STRICT_MODE, default to SupportsAll. + switch strictMode := os.Getenv(envvars.CliStrictMode); strictMode { + case "bot": + acct.SupportedIdentities = credential.SupportsBot + case "user": + acct.SupportedIdentities = credential.SupportsUser + case "off", "": + acct.SupportedIdentities = credential.SupportsAll + default: + return nil, &credential.BlockError{ + Provider: "sidecar", + Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envvars.CliStrictMode, strictMode), + } + } + + return acct, nil +} + +// ResolveToken returns a sentinel token whose value encodes the token type. +// The transport interceptor reads this sentinel to determine the identity +// (user vs bot), strips it, and the sidecar injects the real token. +// Returns nil, nil when sidecar mode is not active. +func (p *Provider) ResolveToken(ctx context.Context, req credential.TokenSpec) (*credential.Token, error) { + if os.Getenv(envvars.CliAuthProxy) == "" { + return nil, nil + } + + var sentinel string + switch req.Type { + case credential.TokenTypeUAT: + sentinel = sidecar.SentinelUAT + case credential.TokenTypeTAT: + sentinel = sidecar.SentinelTAT + default: + return nil, nil + } + + return &credential.Token{ + Value: sentinel, + Scopes: "", // empty → scope pre-check is skipped + Source: "sidecar", + }, nil +} + +func init() { + credential.Register(&Provider{}) +} diff --git a/extension/credential/sidecar/provider_test.go b/extension/credential/sidecar/provider_test.go new file mode 100644 index 000000000..1abdce063 --- /dev/null +++ b/extension/credential/sidecar/provider_test.go @@ -0,0 +1,188 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +package sidecar + +import ( + "context" + "os" + "testing" + + "github.com/larksuite/cli/extension/credential" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/sidecar" +) + +func setEnv(t *testing.T, key, value string) { + t.Helper() + old, hadOld := os.LookupEnv(key) + os.Setenv(key, value) + t.Cleanup(func() { + if hadOld { + os.Setenv(key, old) + } else { + os.Unsetenv(key) + } + }) +} + +func unsetEnv(t *testing.T, key string) { + t.Helper() + old, hadOld := os.LookupEnv(key) + os.Unsetenv(key) + t.Cleanup(func() { + if hadOld { + os.Setenv(key, old) + } + }) +} + +func TestResolveAccount_NotActive(t *testing.T) { + unsetEnv(t, envvars.CliAuthProxy) + + p := &Provider{} + acct, err := p.ResolveAccount(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if acct != nil { + t.Fatal("expected nil account when AUTH_PROXY not set") + } +} + +func TestResolveAccount_Active(t *testing.T) { + setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envvars.CliProxyKey, "test-key") + setEnv(t, envvars.CliAppID, "cli_test123") + setEnv(t, envvars.CliBrand, "lark") + unsetEnv(t, envvars.CliDefaultAs) + unsetEnv(t, envvars.CliStrictMode) + + p := &Provider{} + acct, err := p.ResolveAccount(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if acct == nil { + t.Fatal("expected non-nil account") + } + if acct.AppID != "cli_test123" { + t.Errorf("AppID = %q, want %q", acct.AppID, "cli_test123") + } + if acct.Brand != credential.BrandLark { + t.Errorf("Brand = %q, want %q", acct.Brand, credential.BrandLark) + } + if acct.AppSecret != credential.NoAppSecret { + t.Errorf("AppSecret should be NoAppSecret, got %q", acct.AppSecret) + } + if acct.SupportedIdentities != credential.SupportsAll { + t.Errorf("SupportedIdentities = %d, want %d (SupportsAll)", acct.SupportedIdentities, credential.SupportsAll) + } +} + +func TestResolveAccount_MissingProxyKey(t *testing.T) { + setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") + unsetEnv(t, envvars.CliProxyKey) + setEnv(t, envvars.CliAppID, "cli_test") + + p := &Provider{} + _, err := p.ResolveAccount(context.Background()) + if err == nil { + t.Fatal("expected error when PROXY_KEY is missing") + } + if _, ok := err.(*credential.BlockError); !ok { + t.Fatalf("expected BlockError, got %T: %v", err, err) + } +} + +func TestResolveAccount_MissingAppID(t *testing.T) { + setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envvars.CliProxyKey, "test-key") + unsetEnv(t, envvars.CliAppID) + + p := &Provider{} + _, err := p.ResolveAccount(context.Background()) + if err == nil { + t.Fatal("expected error when APP_ID is missing") + } + if _, ok := err.(*credential.BlockError); !ok { + t.Fatalf("expected BlockError, got %T: %v", err, err) + } +} + +func TestResolveAccount_StrictMode(t *testing.T) { + setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envvars.CliProxyKey, "test-key") + setEnv(t, envvars.CliAppID, "cli_test") + + tests := []struct { + mode string + want credential.IdentitySupport + }{ + {"bot", credential.SupportsBot}, + {"user", credential.SupportsUser}, + {"off", credential.SupportsAll}, + {"", credential.SupportsAll}, + } + + p := &Provider{} + for _, tt := range tests { + t.Run("strict_"+tt.mode, func(t *testing.T) { + if tt.mode == "" { + unsetEnv(t, envvars.CliStrictMode) + } else { + setEnv(t, envvars.CliStrictMode, tt.mode) + } + acct, err := p.ResolveAccount(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if acct.SupportedIdentities != tt.want { + t.Errorf("SupportedIdentities = %d, want %d", acct.SupportedIdentities, tt.want) + } + }) + } +} + +func TestResolveToken_NotActive(t *testing.T) { + unsetEnv(t, envvars.CliAuthProxy) + + p := &Provider{} + tok, err := p.ResolveToken(context.Background(), credential.TokenSpec{Type: credential.TokenTypeUAT}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tok != nil { + t.Fatal("expected nil token when AUTH_PROXY not set") + } +} + +func TestResolveToken_Sentinels(t *testing.T) { + setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envvars.CliProxyKey, "test-key") + + p := &Provider{} + + // UAT + tok, err := p.ResolveToken(context.Background(), credential.TokenSpec{Type: credential.TokenTypeUAT}) + if err != nil { + t.Fatalf("UAT: unexpected error: %v", err) + } + if tok.Value != sidecar.SentinelUAT { + t.Errorf("UAT value = %q, want %q", tok.Value, sidecar.SentinelUAT) + } + if tok.Scopes != "" { + t.Errorf("UAT scopes should be empty, got %q", tok.Scopes) + } + + // TAT + tok, err = p.ResolveToken(context.Background(), credential.TokenSpec{Type: credential.TokenTypeTAT}) + if err != nil { + t.Fatalf("TAT: unexpected error: %v", err) + } + if tok.Value != sidecar.SentinelTAT { + t.Errorf("TAT value = %q, want %q", tok.Value, sidecar.SentinelTAT) + } +} diff --git a/extension/transport/sidecar/interceptor.go b/extension/transport/sidecar/interceptor.go new file mode 100644 index 000000000..6d928bd8a --- /dev/null +++ b/extension/transport/sidecar/interceptor.go @@ -0,0 +1,178 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +// Package sidecar provides a transport interceptor for the auth sidecar +// proxy mode. When LARKSUITE_CLI_AUTH_PROXY is set (an HTTP URL), all +// outgoing requests are rewritten to the sidecar address. The interceptor +// strips placeholder credentials, injects proxy headers, and signs each +// request with HMAC-SHA256. No custom DialContext is needed — Go's +// standard http.Transport connects to the sidecar via plain HTTP. +package sidecar + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "os" + "strings" + + "github.com/larksuite/cli/extension/transport" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/sidecar" +) + +// Provider implements transport.Provider for the sidecar mode. +type Provider struct{} + +func (p *Provider) Name() string { return "sidecar" } + +// ResolveInterceptor returns a SidecarInterceptor when sidecar mode is active. +// Returns nil when sidecar mode is disabled or the proxy address is invalid; +// in the latter case a warning is emitted to stderr and requests fall back to +// the non-sidecar transport path (where the credential layer will typically +// block them for lack of a valid account). +func (p *Provider) ResolveInterceptor(ctx context.Context) transport.Interceptor { + proxyAddr := os.Getenv(envvars.CliAuthProxy) + if proxyAddr == "" { + return nil + } + if err := sidecar.ValidateProxyAddr(proxyAddr); err != nil { + fmt.Fprintf(os.Stderr, "WARNING: invalid %s, sidecar interceptor disabled: %v\n", envvars.CliAuthProxy, err) + return nil + } + key := os.Getenv(envvars.CliProxyKey) + return &Interceptor{ + key: []byte(key), + sidecarHost: sidecar.ProxyHost(proxyAddr), + } +} + +// Interceptor rewrites requests for the sidecar proxy. +type Interceptor struct { + key []byte // HMAC signing key + sidecarHost string // sidecar host:port for URL rewriting +} + +// PreRoundTrip rewrites the request for sidecar routing when it carries a +// sentinel token. Requests without a sentinel token (e.g. pre-signed download +// URLs) are passed through unmodified. +// +// Supports two auth patterns: +// - Standard OpenAPI: Authorization: Bearer +// - MCP protocol: X-Lark-MCP-UAT/TAT: +func (i *Interceptor) PreRoundTrip(req *http.Request) func(resp *http.Response, err error) { + identity, authHeader := detectSentinel(req) + if identity == "" { + return nil // not a sidecar-managed request, pass through + } + + // 1. Buffer the body first, before mutating any request state. A partial + // read would sign a truncated body and cause a misleading HMAC mismatch + // on the sidecar side; bail out early and let the request fall through + // unmodified so the credential layer can surface an actionable error. + var bodyBytes []byte + if req.Body != nil { + var err error + bodyBytes, err = io.ReadAll(req.Body) + _ = req.Body.Close() // release original body (fd/pipe/etc.) after buffering + if err != nil { + fmt.Fprintf(os.Stderr, "WARNING: sidecar interceptor failed to read request body: %v\n", err) + return nil + } + req.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + if req.GetBody != nil { + req.GetBody = func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(bodyBytes)), nil + } + } + } + + // 2. Save original target (scheme://host) + originalScheme := "https" + if req.URL.Scheme != "" { + originalScheme = req.URL.Scheme + } + originalHost := req.URL.Host + req.Header.Set(sidecar.HeaderProxyTarget, originalScheme+"://"+originalHost) + + // 3. Set identity and tell sidecar which header to inject real token into + req.Header.Set(sidecar.HeaderProxyIdentity, identity) + req.Header.Set(sidecar.HeaderProxyAuthHeader, authHeader) + + // 4. Strip placeholder auth header(s) + req.Header.Del("Authorization") + req.Header.Del(sidecar.HeaderMCPUAT) + req.Header.Del(sidecar.HeaderMCPTAT) + + bodySHA := sidecar.BodySHA256(bodyBytes) + req.Header.Set(sidecar.HeaderBodySHA256, bodySHA) + + pathAndQuery := req.URL.RequestURI() + ts := sidecar.Timestamp() + // Cover identity and authHeader in the signature so an on-path attacker + // within the replay window cannot flip the injected token's identity or + // redirect the token into a different header. + sig := sidecar.Sign(i.key, sidecar.CanonicalRequest{ + Version: sidecar.ProtocolV1, + Method: req.Method, + Host: originalHost, + PathAndQuery: pathAndQuery, + BodySHA256: bodySHA, + Timestamp: ts, + Identity: identity, + AuthHeader: authHeader, + }) + req.Header.Set(sidecar.HeaderProxyVersion, sidecar.ProtocolV1) + req.Header.Set(sidecar.HeaderProxyTimestamp, ts) + req.Header.Set(sidecar.HeaderProxySignature, sig) + + // 5. Rewrite URL to route through sidecar + req.URL.Scheme = "http" + req.URL.Host = i.sidecarHost + + return nil // no post-hook needed +} + +// detectSentinel checks both standard Authorization and MCP auth headers for +// sentinel tokens. Returns the identity ("user"/"bot") and the header name +// that carried the sentinel. +// +// Returns ("", "") when the request carries no sentinel token — typically +// requests that require no auth (e.g. pre-signed download URLs where the +// token is embedded in the URL query parameters). +func detectSentinel(req *http.Request) (identity, authHeader string) { + // Check standard Authorization: Bearer + if auth := req.Header.Get("Authorization"); auth != "" { + token := strings.TrimPrefix(auth, "Bearer ") + switch token { + case sidecar.SentinelUAT: + return sidecar.IdentityUser, "Authorization" + case sidecar.SentinelTAT: + return sidecar.IdentityBot, "Authorization" + } + } + // Check MCP headers: X-Lark-MCP-UAT/TAT: + if v := req.Header.Get(sidecar.HeaderMCPUAT); v == sidecar.SentinelUAT { + return sidecar.IdentityUser, sidecar.HeaderMCPUAT + } + if v := req.Header.Get(sidecar.HeaderMCPTAT); v == sidecar.SentinelTAT { + return sidecar.IdentityBot, sidecar.HeaderMCPTAT + } + return "", "" +} + +func init() { + proxyAddr := os.Getenv(envvars.CliAuthProxy) + if proxyAddr == "" { + return + } + if err := sidecar.ValidateProxyAddr(proxyAddr); err != nil { + fmt.Fprintf(os.Stderr, "WARNING: ignoring invalid %s: %v\n", envvars.CliAuthProxy, err) + return + } + transport.Register(&Provider{}) +} diff --git a/extension/transport/sidecar/interceptor_test.go b/extension/transport/sidecar/interceptor_test.go new file mode 100644 index 000000000..2cb0def1f --- /dev/null +++ b/extension/transport/sidecar/interceptor_test.go @@ -0,0 +1,265 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +package sidecar + +import ( + "bytes" + "errors" + "io" + "net/http" + "testing" + + "github.com/larksuite/cli/sidecar" +) + +// failingBody is a ReadCloser that errors on Read and tracks Close calls. +type failingBody struct { + err error + closed bool + readCall bool +} + +func (b *failingBody) Read(p []byte) (int, error) { + b.readCall = true + return 0, b.err +} + +func (b *failingBody) Close() error { + b.closed = true + return nil +} + +func TestInterceptor_PreRoundTrip(t *testing.T) { + key := []byte("test-key-for-hmac-signing-32byte!") + interceptor := &Interceptor{key: key, sidecarHost: "127.0.0.1:16384"} + + body := []byte(`{"msg":"hello"}`) + req, _ := http.NewRequest("POST", "https://open.feishu.cn/open-apis/im/v1/messages?receive_id_type=chat_id", io.NopCloser(bytes.NewReader(body))) + req.Header.Set("Authorization", "Bearer "+sidecar.SentinelUAT) + req.Header.Set("X-Cli-Source", "lark-cli") + + post := interceptor.PreRoundTrip(req) + + if post != nil { + t.Error("expected nil post hook") + } + + // URL should be rewritten to sidecar + if req.URL.Scheme != "http" { + t.Errorf("scheme = %q, want %q", req.URL.Scheme, "http") + } + if req.URL.Host != "127.0.0.1:16384" { + t.Errorf("host = %q, want %q", req.URL.Host, "127.0.0.1:16384") + } + + // Original target should be preserved + target := req.Header.Get(sidecar.HeaderProxyTarget) + if target != "https://open.feishu.cn" { + t.Errorf("target = %q, want %q", target, "https://open.feishu.cn") + } + + // Identity should be user (from SentinelUAT) + if identity := req.Header.Get(sidecar.HeaderProxyIdentity); identity != sidecar.IdentityUser { + t.Errorf("identity = %q, want %q", identity, sidecar.IdentityUser) + } + + // Authorization should be stripped + if auth := req.Header.Get("Authorization"); auth != "" { + t.Errorf("Authorization header should be stripped, got %q", auth) + } + + // HMAC headers should be set + if sig := req.Header.Get(sidecar.HeaderProxySignature); sig == "" { + t.Error("signature header should be set") + } + if ts := req.Header.Get(sidecar.HeaderProxyTimestamp); ts == "" { + t.Error("timestamp header should be set") + } + if sha := req.Header.Get(sidecar.HeaderBodySHA256); sha == "" { + t.Error("body SHA256 header should be set") + } + if v := req.Header.Get(sidecar.HeaderProxyVersion); v != sidecar.ProtocolV1 { + t.Errorf("version header = %q, want %q", v, sidecar.ProtocolV1) + } + + // Non-proxy headers should be preserved + if src := req.Header.Get("X-Cli-Source"); src != "lark-cli" { + t.Errorf("X-Cli-Source should be preserved, got %q", src) + } + + // Body should still be readable + readBody, _ := io.ReadAll(req.Body) + if !bytes.Equal(readBody, body) { + t.Errorf("body should be preserved after PreRoundTrip") + } +} + +func TestInterceptor_BotIdentity(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + req, _ := http.NewRequest("GET", "https://open.feishu.cn/open-apis/calendar/v4/events", nil) + req.Header.Set("Authorization", "Bearer "+sidecar.SentinelTAT) + + interceptor.PreRoundTrip(req) + + if identity := req.Header.Get(sidecar.HeaderProxyIdentity); identity != sidecar.IdentityBot { + t.Errorf("identity = %q, want %q", identity, sidecar.IdentityBot) + } +} + +func TestInterceptor_NonSentinelToken_PassThrough(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + origURL := "https://some-cdn.example.com/presigned-download?token=abc" + req, _ := http.NewRequest("GET", origURL, nil) + req.Header.Set("Authorization", "Bearer some-real-token") + + post := interceptor.PreRoundTrip(req) + + // Should NOT be rewritten — no sentinel token + if post != nil { + t.Error("expected nil post hook for pass-through") + } + if req.URL.String() != origURL { + t.Errorf("URL should be unchanged, got %q", req.URL.String()) + } + if req.Header.Get(sidecar.HeaderProxyTarget) != "" { + t.Error("proxy target header should not be set for pass-through") + } + if req.Header.Get("Authorization") != "Bearer some-real-token" { + t.Error("Authorization should be preserved for pass-through") + } +} + +func TestInterceptor_NoAuth_PassThrough(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + origURL := "https://cdn.feishu.cn/download/file" + req, _ := http.NewRequest("GET", origURL, nil) + + interceptor.PreRoundTrip(req) + + // No Authorization header at all — should pass through + if req.URL.String() != origURL { + t.Errorf("URL should be unchanged for no-auth request, got %q", req.URL.String()) + } +} + +func TestInterceptor_MCP_UAT(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + req, _ := http.NewRequest("POST", "https://mcp.feishu.cn/mcp/v1/tools/call", bytes.NewReader([]byte(`{"jsonrpc":"2.0"}`))) + req.Header.Set(sidecar.HeaderMCPUAT, sidecar.SentinelUAT) + + interceptor.PreRoundTrip(req) + + // Should be intercepted and rewritten + if req.URL.Host != "127.0.0.1:16384" { + t.Errorf("host = %q, want sidecar host", req.URL.Host) + } + if identity := req.Header.Get(sidecar.HeaderProxyIdentity); identity != sidecar.IdentityUser { + t.Errorf("identity = %q, want %q", identity, sidecar.IdentityUser) + } + if ah := req.Header.Get(sidecar.HeaderProxyAuthHeader); ah != sidecar.HeaderMCPUAT { + t.Errorf("auth header = %q, want %q", ah, sidecar.HeaderMCPUAT) + } + // MCP sentinel should be stripped + if v := req.Header.Get(sidecar.HeaderMCPUAT); v != "" { + t.Errorf("MCP-UAT should be stripped, got %q", v) + } +} + +func TestInterceptor_MCP_TAT(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + req, _ := http.NewRequest("POST", "https://mcp.feishu.cn/mcp/v1/tools/call", bytes.NewReader([]byte(`{}`))) + req.Header.Set(sidecar.HeaderMCPTAT, sidecar.SentinelTAT) + + interceptor.PreRoundTrip(req) + + if identity := req.Header.Get(sidecar.HeaderProxyIdentity); identity != sidecar.IdentityBot { + t.Errorf("identity = %q, want %q", identity, sidecar.IdentityBot) + } + if ah := req.Header.Get(sidecar.HeaderProxyAuthHeader); ah != sidecar.HeaderMCPTAT { + t.Errorf("auth header = %q, want %q", ah, sidecar.HeaderMCPTAT) + } +} + +func TestInterceptor_StandardAuth_SetsAuthorizationHeader(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + req, _ := http.NewRequest("GET", "https://open.feishu.cn/open-apis/test", nil) + req.Header.Set("Authorization", "Bearer "+sidecar.SentinelUAT) + + interceptor.PreRoundTrip(req) + + if ah := req.Header.Get(sidecar.HeaderProxyAuthHeader); ah != "Authorization" { + t.Errorf("auth header = %q, want %q", ah, "Authorization") + } +} + +// TestInterceptor_BodyReadError verifies that when io.ReadAll on the request +// body fails partway, PreRoundTrip skips the rewrite entirely rather than +// signing a truncated body (which would produce a misleading HMAC mismatch on +// the sidecar side) and releases the original body. +func TestInterceptor_BodyReadError(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + const origURL = "https://open.feishu.cn/open-apis/im/v1/messages" + body := &failingBody{err: errors.New("disk gremlin")} + + req, _ := http.NewRequest("POST", origURL, body) + req.Header.Set("Authorization", "Bearer "+sidecar.SentinelUAT) + + post := interceptor.PreRoundTrip(req) + + if post != nil { + t.Error("expected nil post hook on body read failure") + } + + // Original body must be closed to avoid leaking fd/pipe-like resources. + if !body.readCall { + t.Error("expected ReadAll to have attempted reading from the body") + } + if !body.closed { + t.Error("expected original body to be Close()'d after read failure") + } + + // URL must NOT be rewritten — request should fall through to the next + // layer (credential) which can surface a meaningful error. + if req.URL.String() != origURL { + t.Errorf("URL should be unchanged on read failure, got %q", req.URL.String()) + } + + // No proxy/HMAC headers should leak onto the request. + for _, h := range []string{ + sidecar.HeaderProxyVersion, + sidecar.HeaderProxyTarget, + sidecar.HeaderProxySignature, + sidecar.HeaderProxyTimestamp, + sidecar.HeaderBodySHA256, + sidecar.HeaderProxyIdentity, + sidecar.HeaderProxyAuthHeader, + } { + if v := req.Header.Get(h); v != "" { + t.Errorf("%s should not be set on read failure, got %q", h, v) + } + } +} + +func TestInterceptor_EmptyBody(t *testing.T) { + interceptor := &Interceptor{key: []byte("key"), sidecarHost: "127.0.0.1:16384"} + + req, _ := http.NewRequest("GET", "https://open.feishu.cn/path", nil) + req.Header.Set("Authorization", "Bearer "+sidecar.SentinelTAT) + interceptor.PreRoundTrip(req) + + sha := req.Header.Get(sidecar.HeaderBodySHA256) + expectedEmpty := sidecar.BodySHA256(nil) + if sha != expectedEmpty { + t.Errorf("body SHA256 = %q, want empty-string SHA256 %q", sha, expectedEmpty) + } +} diff --git a/internal/envvars/envvars.go b/internal/envvars/envvars.go index 1d80ac1cc..ecb629fd0 100644 --- a/internal/envvars/envvars.go +++ b/internal/envvars/envvars.go @@ -11,4 +11,8 @@ const ( CliTenantAccessToken = "LARKSUITE_CLI_TENANT_ACCESS_TOKEN" CliDefaultAs = "LARKSUITE_CLI_DEFAULT_AS" CliStrictMode = "LARKSUITE_CLI_STRICT_MODE" + + // Sidecar proxy (auth proxy mode) + CliAuthProxy = "LARKSUITE_CLI_AUTH_PROXY" // sidecar HTTP address, e.g. "http://127.0.0.1:16384" + CliProxyKey = "LARKSUITE_CLI_PROXY_KEY" // HMAC signing key shared with sidecar ) diff --git a/main_authsidecar.go b/main_authsidecar.go new file mode 100644 index 000000000..039180a2e --- /dev/null +++ b/main_authsidecar.go @@ -0,0 +1,11 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +package main + +import ( + _ "github.com/larksuite/cli/extension/credential/sidecar" // activate sidecar credential provider + _ "github.com/larksuite/cli/extension/transport/sidecar" // activate sidecar transport interceptor +) diff --git a/main_noauthsidecar.go b/main_noauthsidecar.go new file mode 100644 index 000000000..514afda63 --- /dev/null +++ b/main_noauthsidecar.go @@ -0,0 +1,54 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build !authsidecar + +// This file is the fail-closed guard for builds that do NOT include the +// `authsidecar` tag. The sidecar credential-isolation feature is only +// compiled in under that tag; deploying the plain build into an environment +// that expects sidecar isolation would silently fall back to direct env +// credential use — exactly the failure mode the feature is meant to prevent. +// +// When LARKSUITE_CLI_AUTH_PROXY is set, we refuse to run rather than ignore +// the variable. The operator either rebuilt without realizing (wrong +// artifact) or the sandbox inherited the var by accident; both cases want +// a loud startup error, not a mysterious token leak on the first API call. + +package main + +import ( + "fmt" + "io" + "os" + + "github.com/larksuite/cli/internal/envvars" +) + +func init() { + if code := checkNoAuthsidecarBuild(os.Getenv, os.Stderr); code != 0 { + os.Exit(code) + } +} + +// checkNoAuthsidecarBuild returns a non-zero exit code (and writes a +// human-readable reason to stderr) when the environment asks for sidecar +// isolation that this binary cannot provide. Factored out from init() so +// tests can exercise the decision without actually calling os.Exit. +func checkNoAuthsidecarBuild(getenv func(string) string, stderr io.Writer) int { + v := getenv(envvars.CliAuthProxy) + if v == "" { + return 0 + } + fmt.Fprintf(stderr, + "ERROR: %s is set, but this lark-cli binary was built WITHOUT the "+ + "'authsidecar' build tag.\n"+ + "The sidecar credential-isolation feature is compiled out — "+ + "running would bypass isolation and\n"+ + "send any real credentials present in the environment directly "+ + "to the Lark API.\n\n"+ + "To fix, either:\n"+ + " - rebuild the CLI with: go build -tags authsidecar\n"+ + " - or unset %s if sidecar isolation is not required\n", + envvars.CliAuthProxy, envvars.CliAuthProxy) + return 2 +} diff --git a/main_noauthsidecar_test.go b/main_noauthsidecar_test.go new file mode 100644 index 000000000..5879015c2 --- /dev/null +++ b/main_noauthsidecar_test.go @@ -0,0 +1,52 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build !authsidecar + +package main + +import ( + "bytes" + "strings" + "testing" + + "github.com/larksuite/cli/internal/envvars" +) + +func TestCheckNoAuthsidecarBuild_Unset(t *testing.T) { + var stderr bytes.Buffer + code := checkNoAuthsidecarBuild(func(string) string { return "" }, &stderr) + if code != 0 { + t.Errorf("exit code = %d, want 0 when AUTH_PROXY is unset", code) + } + if stderr.Len() != 0 { + t.Errorf("stderr should be empty, got %q", stderr.String()) + } +} + +// TestCheckNoAuthsidecarBuild_Set verifies that deploying a plain build into +// a sandbox that expects sidecar isolation fails loudly at startup instead +// of silently leaking credentials through the env provider path. +func TestCheckNoAuthsidecarBuild_Set(t *testing.T) { + var stderr bytes.Buffer + env := func(k string) string { + if k == envvars.CliAuthProxy { + return "http://127.0.0.1:16384" + } + return "" + } + code := checkNoAuthsidecarBuild(env, &stderr) + if code == 0 { + t.Fatal("expected non-zero exit code when AUTH_PROXY is set") + } + msg := stderr.String() + for _, want := range []string{ + envvars.CliAuthProxy, + "authsidecar", // build-tag name must appear so operators can act on it + "rebuild", + } { + if !strings.Contains(msg, want) { + t.Errorf("stderr message missing %q; got:\n%s", want, msg) + } + } +} diff --git a/sidecar/hmac.go b/sidecar/hmac.go new file mode 100644 index 000000000..2c2f58759 --- /dev/null +++ b/sidecar/hmac.go @@ -0,0 +1,88 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package sidecar + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "fmt" + "math" + "strconv" + "strings" + "time" +) + +// BodySHA256 returns the hex-encoded SHA-256 digest of body. +// An empty or nil body produces the SHA-256 of the empty string. +func BodySHA256(body []byte) string { + h := sha256.Sum256(body) + return hex.EncodeToString(h[:]) +} + +// CanonicalRequest is the set of fields covered by the HMAC signature. +// Clients and servers must populate every field identically for verification +// to succeed; any field that is forwarded but *not* covered by this struct can +// be tampered with inside the MaxTimestampDrift replay window without +// invalidating the signature. +// +// Version must be set to a known protocol constant (ProtocolV1). It is the +// first field in the canonical string so that a future v2 with different +// structure cannot be confused for v1 output under the same key. +type CanonicalRequest struct { + Version string // e.g. ProtocolV1 + Method string // e.g. "GET", "POST" + Host string // e.g. "open.feishu.cn" + PathAndQuery string // e.g. "/open-apis/calendar/v4/events?page_size=50" + BodySHA256 string // hex-encoded SHA-256 of the request body + Timestamp string // Unix epoch seconds string + Identity string // IdentityUser or IdentityBot + AuthHeader string // header the server should inject the real token into +} + +// canonicalString joins the fields with newlines. Field order is part of the +// protocol contract — do not reorder without bumping Version. +func (c CanonicalRequest) canonicalString() string { + return strings.Join([]string{ + c.Version, + c.Method, + c.Host, + c.PathAndQuery, + c.BodySHA256, + c.Timestamp, + c.Identity, + c.AuthHeader, + }, "\n") +} + +// Sign computes the HMAC-SHA256 signature over the canonical request string. +func Sign(key []byte, req CanonicalRequest) string { + mac := hmac.New(sha256.New, key) + mac.Write([]byte(req.canonicalString())) + return hex.EncodeToString(mac.Sum(nil)) +} + +// Verify checks that signature matches the HMAC-SHA256 of the canonical +// request and that the timestamp is within MaxTimestampDrift seconds of now. +// Returns nil on success. +func Verify(key []byte, req CanonicalRequest, signature string) error { + ts, err := strconv.ParseInt(req.Timestamp, 10, 64) + if err != nil { + return fmt.Errorf("invalid timestamp %q: %w", req.Timestamp, err) + } + drift := math.Abs(float64(time.Now().Unix() - ts)) + if drift > MaxTimestampDrift { + return fmt.Errorf("timestamp drift %.0fs exceeds limit %ds", drift, MaxTimestampDrift) + } + expected := Sign(key, req) + if !hmac.Equal([]byte(expected), []byte(signature)) { + return fmt.Errorf("HMAC signature mismatch") + } + return nil +} + +// Timestamp returns the current Unix epoch seconds as a string. +func Timestamp() string { + return strconv.FormatInt(time.Now().Unix(), 10) +} diff --git a/sidecar/hmac_test.go b/sidecar/hmac_test.go new file mode 100644 index 000000000..f5a691c2e --- /dev/null +++ b/sidecar/hmac_test.go @@ -0,0 +1,300 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package sidecar + +import ( + "strconv" + "strings" + "testing" + "time" +) + +func TestBodySHA256_Empty(t *testing.T) { + // SHA-256 of empty string is a well-known constant. + want := "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + if got := BodySHA256(nil); got != want { + t.Errorf("BodySHA256(nil) = %q, want %q", got, want) + } + if got := BodySHA256([]byte{}); got != want { + t.Errorf("BodySHA256([]byte{}) = %q, want %q", got, want) + } +} + +func TestBodySHA256_NonEmpty(t *testing.T) { + got := BodySHA256([]byte(`{"key":"value"}`)) + if len(got) != 64 { + t.Errorf("expected 64-char hex string, got %d chars", len(got)) + } +} + +// canonical is a test helper that builds a fully-populated CanonicalRequest +// with reasonable defaults, so individual tests can override just the field +// they want to tamper with. +func canonical(override func(*CanonicalRequest)) CanonicalRequest { + c := CanonicalRequest{ + Version: ProtocolV1, + Method: "POST", + Host: "open.feishu.cn", + PathAndQuery: "/open-apis/im/v1/messages?receive_id_type=chat_id", + BodySHA256: BodySHA256([]byte(`{"content":"hello"}`)), + Timestamp: Timestamp(), + Identity: IdentityUser, + AuthHeader: "Authorization", + } + if override != nil { + override(&c) + } + return c +} + +func TestSignAndVerify(t *testing.T) { + key := []byte("test-secret-key-32bytes-long!!!!!") + req := canonical(nil) + + sig := Sign(key, req) + if len(sig) != 64 { + t.Fatalf("signature should be 64-char hex, got %d chars", len(sig)) + } + + // Valid verification + if err := Verify(key, req, sig); err != nil { + t.Fatalf("Verify failed for valid signature: %v", err) + } + + // Wrong key + if err := Verify([]byte("wrong-key"), req, sig); err == nil { + t.Error("Verify should fail with wrong key") + } + + // Each field must be covered by the signature — tampering with any one + // invalidates it. + fields := map[string]func(*CanonicalRequest){ + "version": func(c *CanonicalRequest) { c.Version = "v2" }, + "method": func(c *CanonicalRequest) { c.Method = "GET" }, + "host": func(c *CanonicalRequest) { c.Host = "evil.com" }, + "pathAndQuery": func(c *CanonicalRequest) { c.PathAndQuery = "/steal" }, + "bodySHA256": func(c *CanonicalRequest) { c.BodySHA256 = BodySHA256([]byte("tampered")) }, + "identity": func(c *CanonicalRequest) { c.Identity = IdentityBot }, + "authHeader": func(c *CanonicalRequest) { c.AuthHeader = "Cookie" }, + } + for name, mutate := range fields { + t.Run("tamper_"+name, func(t *testing.T) { + tampered := canonical(mutate) + if err := Verify(key, tampered, sig); err == nil { + t.Errorf("Verify should fail when %s is tampered", name) + } + }) + } +} + +// TestVerify_PrivilegeConfusion proves C1: without identity and authHeader in +// the canonical string, an attacker holding a captured user-signed request +// could replay it as bot (or vice versa) by flipping the header. With both +// fields now covered, such a flip must invalidate the signature. +func TestVerify_PrivilegeConfusion(t *testing.T) { + key := []byte("test-key") + signed := canonical(func(c *CanonicalRequest) { c.Identity = IdentityUser }) + sig := Sign(key, signed) + + replayed := signed + replayed.Identity = IdentityBot // attacker flips identity + if err := Verify(key, replayed, sig); err == nil { + t.Error("identity flip must invalidate signature") + } + + replayed = signed + replayed.AuthHeader = "Cookie" // attacker redirects injection target + if err := Verify(key, replayed, sig); err == nil { + t.Error("auth-header flip must invalidate signature") + } +} + +func TestVerify_TimestampDrift(t *testing.T) { + key := []byte("test-key") + + // Timestamp too old + oldTs := strconv.FormatInt(time.Now().Unix()-MaxTimestampDrift-10, 10) + oldReq := canonical(func(c *CanonicalRequest) { c.Timestamp = oldTs }) + sig := Sign(key, oldReq) + if err := Verify(key, oldReq, sig); err == nil { + t.Error("Verify should reject expired timestamp") + } + + // Timestamp too far in future + futureTs := strconv.FormatInt(time.Now().Unix()+MaxTimestampDrift+10, 10) + futureReq := canonical(func(c *CanonicalRequest) { c.Timestamp = futureTs }) + sig = Sign(key, futureReq) + if err := Verify(key, futureReq, sig); err == nil { + t.Error("Verify should reject future timestamp") + } + + // Invalid timestamp + badTs := canonical(func(c *CanonicalRequest) { c.Timestamp = "not-a-number" }) + if err := Verify(key, badTs, "sig"); err == nil { + t.Error("Verify should reject invalid timestamp") + } +} + +func TestSignDeterministic(t *testing.T) { + key := []byte("key") + req := canonical(func(c *CanonicalRequest) { c.Timestamp = "12345" }) + a, b := Sign(key, req), Sign(key, req) + if a != b { + t.Errorf("Sign should be deterministic: %q vs %q", a, b) + } +} + +func TestValidateProxyAddr(t *testing.T) { + valid := []string{ + // loopback IPs + "http://127.0.0.1:16384", + "127.0.0.1:16384", + "[::1]:16384", + "http://[::1]:16384", + // recognized same-host aliases + "http://localhost:8080", + "localhost:8080", + "http://host.docker.internal:16384", + "http://host.containers.internal:16384", + "http://host.lima.internal:16384", + "http://gateway.docker.internal:16384", + // trailing slash is tolerated + "http://127.0.0.1:8080/", + } + for _, addr := range valid { + if err := ValidateProxyAddr(addr); err != nil { + t.Errorf("ValidateProxyAddr(%q) unexpected error: %v", addr, err) + } + } + + invalid := []string{ + "", + "foobar", + "ftp://127.0.0.1:16384", + "http://", + "http://127.0.0.1:16384/some/path", + ":16384", + } + for _, addr := range invalid { + if err := ValidateProxyAddr(addr); err == nil { + t.Errorf("ValidateProxyAddr(%q) expected error, got nil", addr) + } + } +} + +// TestValidateProxyAddr_HostConstraint pins C2: the sidecar pattern is +// same-machine by definition, so the validator rejects any host that isn't +// loopback or a recognized same-host alias. Tampered /etc/hosts is out of +// scope (attacker already has ambient host access). +func TestValidateProxyAddr_HostConstraint(t *testing.T) { + sameHost := []string{ + "http://127.0.0.1:16384", + "http://localhost:8080", + "http://host.docker.internal:16384", + "http://host.containers.internal:16384", + "http://host.lima.internal:16384", + "http://gateway.docker.internal:16384", + "http://[::1]:16384", + // bare form + "127.0.0.1:16384", + "localhost:8080", + "host.docker.internal:16384", + } + for _, addr := range sameHost { + if err := ValidateProxyAddr(addr); err != nil { + t.Errorf("expected %q to pass as same-host, got: %v", addr, err) + } + } + + notSameHost := map[string]string{ + // The interesting ones — plausible misconfigurations / attacks + "public DNS name": "http://attacker.com:8080", + "cloud metadata IMDS": "http://169.254.169.254", + "private RFC1918": "http://10.0.0.1:16384", + "other RFC1918": "http://192.168.1.2:16384", + "link-local IPv4": "http://169.254.1.1:16384", + "unspecified IPv4 (0.0.0.0)": "http://0.0.0.0:16384", + "bare public IP": "http://8.8.8.8:16384", + "bare RFC1918": "10.0.0.1:16384", + } + for name, addr := range notSameHost { + t.Run(name, func(t *testing.T) { + err := ValidateProxyAddr(addr) + if err == nil { + t.Fatalf("expected rejection for %q", addr) + } + // Error must name the constraint so users know why. + msg := err.Error() + if !strings.Contains(msg, "loopback") && !strings.Contains(msg, "same-host") { + t.Errorf("error should explain same-host requirement, got: %v", err) + } + }) + } +} + +// TestValidateProxyAddr_RejectsUserinfo closes the URL-phishing vector +// http://127.0.0.1@attacker.com (where "127.0.0.1" is actually basic-auth +// userinfo and the real host is attacker.com). userinfo has no legitimate +// use in the sidecar protocol. +func TestValidateProxyAddr_RejectsUserinfo(t *testing.T) { + for _, addr := range []string{ + "http://user@127.0.0.1:16384", + "http://user:pass@127.0.0.1:16384", + "http://127.0.0.1@attacker.com:16384", + } { + err := ValidateProxyAddr(addr) + if err == nil { + t.Errorf("ValidateProxyAddr(%q): expected rejection, got nil", addr) + continue + } + // Either "userinfo" (for addresses parsed with user) or the same-host + // message (for e.g. http://127.0.0.1@attacker.com where the REAL + // host parses as attacker.com) is acceptable — both reject the + // phishing attempt. + msg := err.Error() + if !strings.Contains(msg, "userinfo") && !strings.Contains(msg, "same-host") && !strings.Contains(msg, "loopback") { + t.Errorf("error should reject userinfo or flag wrong host, got: %v", err) + } + } +} + +// TestValidateProxyAddr_HTTPSRejected pins the current contract: https is +// rejected explicitly (not lumped into a generic "bad scheme" error) because +// the interceptor hardcodes http and would silently downgrade an https URL +// otherwise. The message must mention https so users understand why their +// perfectly-looking config is refused. +func TestValidateProxyAddr_HTTPSRejected(t *testing.T) { + for _, addr := range []string{ + "https://127.0.0.1:16384", + "https://sidecar.corp.internal:443", + } { + err := ValidateProxyAddr(addr) + if err == nil { + t.Errorf("ValidateProxyAddr(%q): expected error, got nil", addr) + continue + } + if !strings.Contains(err.Error(), "https") { + t.Errorf("ValidateProxyAddr(%q): error should mention https, got: %v", addr, err) + } + } +} + +func TestProxyHost(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"http://127.0.0.1:16384", "127.0.0.1:16384"}, + {"http://0.0.0.0:8080", "0.0.0.0:8080"}, + {"http://host.docker.internal:16384/", "host.docker.internal:16384"}, + {"127.0.0.1:16384", "127.0.0.1:16384"}, // no scheme + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + if got := ProxyHost(tt.input); got != tt.want { + t.Errorf("ProxyHost(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} diff --git a/sidecar/protocol.go b/sidecar/protocol.go new file mode 100644 index 000000000..4b8f9c62f --- /dev/null +++ b/sidecar/protocol.go @@ -0,0 +1,198 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package sidecar defines the wire protocol shared between the CLI client +// (running inside a sandbox) and the auth sidecar proxy (running in a +// trusted environment). Communication uses plain HTTP. +package sidecar + +import ( + "fmt" + "net" + "net/url" + "strings" +) + +// ProtocolV1 is the wire-protocol version string embedded in every signed +// request. Servers must reject requests whose HeaderProxyVersion is not a +// version they understand. Bump this constant (and update the canonical +// string) for any breaking change to signing inputs. +const ProtocolV1 = "v1" + +// Proxy request headers set by the CLI transport interceptor. +const ( + // HeaderProxyVersion carries the wire-protocol version (e.g. ProtocolV1). + // Servers must reject requests whose version they do not understand. The + // value is also included in the canonical signing string so that a request + // signed for one version cannot be replayed as another. + HeaderProxyVersion = "X-Lark-Proxy-Version" + + // HeaderProxyTarget carries the original request host (e.g. "open.feishu.cn"). + HeaderProxyTarget = "X-Lark-Proxy-Target" + + // HeaderProxyIdentity carries the resolved identity type ("user" or "bot"). + HeaderProxyIdentity = "X-Lark-Proxy-Identity" + + // HeaderProxySignature carries the HMAC-SHA256 hex signature. + HeaderProxySignature = "X-Lark-Proxy-Signature" + + // HeaderProxyTimestamp carries the Unix epoch seconds string used in signing. + HeaderProxyTimestamp = "X-Lark-Proxy-Timestamp" + + // HeaderBodySHA256 carries the hex-encoded SHA-256 digest of the request body. + HeaderBodySHA256 = "X-Lark-Body-SHA256" + + // HeaderProxyAuthHeader tells the sidecar which header to inject the real + // token into. Defaults to "Authorization" for standard OpenAPI requests. + // MCP requests use "X-Lark-MCP-UAT" or "X-Lark-MCP-TAT". + HeaderProxyAuthHeader = "X-Lark-Proxy-Auth-Header" +) + +// MCP auth headers used by the Lark MCP protocol. +const ( + HeaderMCPUAT = "X-Lark-MCP-UAT" + HeaderMCPTAT = "X-Lark-MCP-TAT" +) + +// Sentinel token values returned by the noop credential provider. +// These are placeholder strings that flow through the SDK auth pipeline +// but are stripped by the transport interceptor before reaching the sidecar. +const ( + SentinelUAT = "sidecar-managed-uat" // User Access Token placeholder + SentinelTAT = "sidecar-managed-tat" // Tenant Access Token placeholder +) + +// IdentityUser and IdentityBot are the wire values for HeaderProxyIdentity. +const ( + IdentityUser = "user" + IdentityBot = "bot" +) + +// MaxTimestampDrift is the maximum allowed difference (in seconds) between +// the request timestamp and the server's current time. +const MaxTimestampDrift = 60 + +// DefaultListenAddr is the default sidecar listen address (localhost only). +const DefaultListenAddr = "127.0.0.1:16384" + +// sameHostAliases names DNS aliases commonly used to reach the host running +// the sandbox across a container / VM boundary. Traffic to these names stays +// on the physical machine (via a virtual bridge), so a plaintext sidecar +// channel still satisfies the sidecar pattern's same-host confidentiality +// requirement. Adding to this list has real security implications — only add +// names that are universally same-host by the runtime's design. +var sameHostAliases = map[string]bool{ + "localhost": true, // universal + "host.docker.internal": true, // Docker Desktop (macOS / Windows) + "host.containers.internal": true, // Podman Desktop + "host.lima.internal": true, // Lima / colima / rancher-desktop + "gateway.docker.internal": true, // Docker Desktop alt name +} + +// isSameHost returns true when host is either a loopback IP or a recognized +// same-host DNS alias. Does not perform DNS resolution — a tampered /etc/hosts +// that points an alias elsewhere is out of scope (attacker with that access +// already has ambient control of the machine). +func isSameHost(host string) bool { + if sameHostAliases[host] { + return true + } + if ip := net.ParseIP(host); ip != nil { + return ip.IsLoopback() + } + return false +} + +// errNotSameHost is the shared error returned when the sidecar address does +// not resolve to the same physical host as the sandbox. Kept in one place so +// tests can look for a stable marker. +func errNotSameHost(addr string) error { + return fmt.Errorf("invalid proxy address %q: host must be loopback "+ + "(127.0.0.1 / ::1) or a recognized same-host alias "+ + "(localhost, host.docker.internal, host.containers.internal, "+ + "host.lima.internal, gateway.docker.internal). "+ + "The sidecar must run on the same physical machine as the sandbox — "+ + "cross-machine deployment is not a sidecar and is not supported", addr) +} + +// ValidateProxyAddr validates the LARKSUITE_CLI_AUTH_PROXY value. +// Accepted formats: +// - http://host:port +// - host:port (bare address, treated as http) +// +// Host must be loopback or in sameHostAliases. The sidecar pattern is +// inherently same-machine; cross-machine deployment is a different product +// and is not supported by this feature. +// +// https:// is rejected because sidecar is a same-host pattern: loopback +// and virtual same-host bridges don't traverse any untrusted medium, so +// TLS adds no security. Cross-machine deployment is out of scope (see the +// host constraint above), so there is no scenario today where https +// provides a real benefit over http on loopback. +// +// userinfo (user:pass@) is rejected unconditionally — the sidecar protocol +// does not use basic auth, and the syntactic slot exists only as a phishing +// vector (e.g. http://127.0.0.1@attacker.com). +// +// Returns an error if the value is not a valid proxy address. +func ValidateProxyAddr(addr string) error { + if addr == "" { + return fmt.Errorf("proxy address is empty") + } + + // Bare host:port (no scheme) — validate as a net address. + if !strings.Contains(addr, "://") { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return fmt.Errorf("invalid proxy address %q: expected host:port or http://host:port", addr) + } + if host == "" || port == "" { + return fmt.Errorf("invalid proxy address %q: host and port must not be empty", addr) + } + if !isSameHost(host) { + return errNotSameHost(addr) + } + return nil + } + + u, err := url.Parse(addr) + if err != nil { + return fmt.Errorf("invalid proxy address %q: %w", addr, err) + } + if u.User != nil { + return fmt.Errorf("invalid proxy address %q: userinfo is not allowed", addr) + } + if u.Scheme == "https" { + return fmt.Errorf("invalid proxy address %q: use http:// — sidecar is "+ + "same-host only (loopback or virtual same-host bridge), so TLS adds "+ + "no security; cross-machine deployment is out of scope", addr) + } + if u.Scheme != "http" { + return fmt.Errorf("invalid proxy address %q: scheme must be http", addr) + } + if u.Host == "" { + return fmt.Errorf("invalid proxy address %q: missing host", addr) + } + if u.Path != "" && u.Path != "/" { + return fmt.Errorf("invalid proxy address %q: path is not allowed", addr) + } + // u.Hostname() strips the port and unwraps IPv6 brackets. + if !isSameHost(u.Hostname()) { + return errNotSameHost(addr) + } + return nil +} + +// ProxyHost extracts the host:port from an AUTH_PROXY URL. +// Input is expected to be an HTTP URL like "http://127.0.0.1:16384". +// Returns the host:port portion for URL rewriting. +func ProxyHost(authProxy string) string { + // Strip scheme + host := authProxy + if i := strings.Index(host, "://"); i >= 0 { + host = host[i+3:] + } + // Strip trailing slash + host = strings.TrimRight(host, "/") + return host +} diff --git a/sidecar/server-demo/README.md b/sidecar/server-demo/README.md new file mode 100644 index 000000000..f0860a44c --- /dev/null +++ b/sidecar/server-demo/README.md @@ -0,0 +1,197 @@ +# Sidecar Server Reference Implementation + +> ⚠️ **This is a demo.** For production deployment, implement your own sidecar +> server conforming to the wire protocol in `github.com/larksuite/cli/sidecar`. + +This example shows how to implement a sidecar auth proxy server that receives +HMAC-signed requests from lark-cli sandbox clients and forwards them to the +Lark/Feishu API with real credentials injected. + +## What this demo shows + +- HMAC-SHA256 request verification (timestamp drift, body digest, signature) +- Target host allowlist + https-only target validation (anti-SSRF / anti-downgrade) +- Identity-based token resolution (UAT for user, TAT for bot) +- Auth-header allowlist: real token may only be injected into `Authorization` + / `X-Lark-MCP-UAT` / `X-Lark-MCP-TAT`, rejecting attempts to smuggle it into + `Cookie`, `User-Agent`, or other intermediate-logged headers +- Audit logging with path ID-segment sanitization and upstream error truncation +- Safe request forwarding (strips client-supplied auth headers) + +## What this demo does NOT handle + +- **TAT refresh** — the shared `DefaultTokenProvider` caches the TAT via + `sync.Once`, which never refreshes. A long-running server will return an + expired TAT after 2 hours. Production implementations should maintain a + TTL-based cache with early renewal. +- High availability / load balancing / hot key rotation +- TLS termination +- Rate limiting / per-identity quotas + +## Both sides need the right build tags + +Sidecar is split into **two separate binaries** with **different build tags**: + +| Side | Binary | Build tag | How to build | +| --- | --- | --- | --- | +| Sandbox (client) | `lark-cli` | `authsidecar` | `go build -tags authsidecar -o lark-cli .` | +| Trusted (server) | `sidecar-server-demo` | `authsidecar_demo` | `go build -tags authsidecar_demo -o sidecar-server-demo ./sidecar/server-demo/` | + +If the sandbox runs a standard `lark-cli` **without** `-tags authsidecar`, the +`LARKSUITE_CLI_AUTH_PROXY` env var is ignored and requests bypass the sidecar +entirely — real credentials (if any) leak to the sandbox. + +## Prerequisites + +The demo reuses the lark-cli credential pipeline, so the trusted machine must +have an app configured: + +```bash +lark-cli config init --new # configure app_id / app_secret (required) +lark-cli auth login # store user refresh_token in keychain + # (only required if sandbox will use --as user) +``` + +`auth login` is **only required for user identity**. If the server will only +serve bot requests (TAT), `config init` alone is enough because the TAT is +minted from `app_id + app_secret`. + +Also, the server process **must not** inherit `LARKSUITE_CLI_AUTH_PROXY` — if +it does, the sidecar credential provider would activate inside the server and +return sentinel tokens instead of real ones. The demo rejects this at startup +with a clear error, but you should make sure to `unset LARKSUITE_CLI_AUTH_PROXY` +in the server shell before launching. + +## Run + +```bash +./sidecar-server-demo \ + --listen 127.0.0.1:16384 \ + --key-file /.lark-sidecar/proxy.key \ + --log-file /.lark-sidecar/audit.log +``` + +### Flags + +| Flag | Default | Purpose | +| --- | --- | --- | +| `--listen` | `127.0.0.1:16384` | Address to bind the HTTP listener | +| `--key-file` | `/.lark-sidecar/proxy.key` | Path to write the generated HMAC key (mode 0600) | +| `--log-file` | *(empty, stderr)* | Audit log output path | +| `--profile` | *(empty, active profile)* | lark-cli profile name for credential lookup | + +### Startup output + +``` +Auth sidecar listening on http://127.0.0.1:16384 +HMAC key prefix: a3b2c1d4 +Full key written to /Users/alice/.lark-sidecar/proxy.key (mode 0600) + +Set in sandbox: + export LARKSUITE_CLI_AUTH_PROXY="http://127.0.0.1:16384" + export LARKSUITE_CLI_PROXY_KEY="" + export LARKSUITE_CLI_APP_ID="cli_xxx" + export LARKSUITE_CLI_BRAND="feishu" +``` + +The `key-file` path is printed exactly as passed on the command line (relative +paths stay relative). The `HMAC key prefix` is the first 8 characters for +identification without revealing the full key. + +### Sandbox env vars (complete list) + +The startup banner only prints the *required* variables. Two more are +optional: + +```bash +export LARKSUITE_CLI_AUTH_PROXY="http://..." # required (see constraints below) +export LARKSUITE_CLI_PROXY_KEY="..." # required +export LARKSUITE_CLI_APP_ID="cli_xxx" # required +export LARKSUITE_CLI_BRAND="feishu" # required (feishu | lark) +export LARKSUITE_CLI_DEFAULT_AS="user" # optional: force default identity +export LARKSUITE_CLI_STRICT_MODE="user" # optional: lock sandbox to one identity +``` + +**`LARKSUITE_CLI_AUTH_PROXY` constraints** — validated by the CLI on startup: + +- Scheme must be `http://` (or bare `host:port`). `https://` is rejected + today because the interceptor does not yet perform TLS; a future PR that + wires up real TLS will relax this. +- Host must be loopback (`127.0.0.1`, `::1`) or one of the recognized + same-host aliases: `localhost`, `host.docker.internal`, + `host.containers.internal`, `host.lima.internal`, `gateway.docker.internal`. + The sidecar pattern is inherently same-machine; cross-machine deployment + is a different product (auth broker / STS) with different security + requirements (mTLS, cert rotation, per-client keys) and is not supported + by this feature. +- No path, query, fragment, or `user:pass@` in the URL. + +**How auto identity detection works in sidecar mode**: on every invocation the +CLI asks the sidecar to look up the logged-in user's `open_id` via +`/open-apis/authen/v1/user_info`. If that succeeds, `--as` defaults to `user`; +if it fails (trusted side has no valid user login, or the call errors out), +it falls back to `bot`. Setting `LARKSUITE_CLI_DEFAULT_AS=user` lets you +short-circuit this and always default to user regardless of the lookup +result; set it to `bot` for the opposite. + +**Note**: `LARKSUITE_CLI_STRICT_MODE` and the server's identity allowlist are +two separate enforcement points: +- `STRICT_MODE` is interpreted locally by the sandbox CLI — it rejects + `--as` values the sandbox itself disallows, before any request goes out. +- The server's allowlist is built from the **trusted-side** config's + `SupportedIdentities` (`sidecar/server-demo/allowlist.go`). The sandbox + cannot override it. + +A well-configured deployment aligns both (e.g. both set to `user` when the +app only supports user tokens), but they are computed independently. + +### Graceful shutdown + +Send `SIGINT` (`Ctrl+C`) or `SIGTERM` to stop the server. The demo drains +in-flight requests with a 5-second timeout before exiting. + +## Wire protocol + +See the [`sidecar` package on pkg.go.dev](https://pkg.go.dev/github.com/larksuite/cli/sidecar) +for protocol constants, HMAC signing/verification, and address validation utilities. + +Headers (client → server): + +| Header | Purpose | +| --- | --- | +| `X-Lark-Proxy-Version` | Wire-protocol version (currently `"v1"`). Server rejects unknown values with 400. | +| `X-Lark-Proxy-Target` | Original target **scheme + host only** (e.g. `https://open.feishu.cn`). Must be `https://`; any path/query/fragment/userinfo in this header is rejected. The path and query come from the request line itself; the server reconstructs the upstream URL as `https:// + requestURI`. | +| `X-Lark-Proxy-Identity` | `"user"` or `"bot"`. Covered by the signature. | +| `X-Lark-Proxy-Auth-Header` | Which header the server should inject real token into. Covered by the signature. | +| `X-Lark-Proxy-Signature` | hex-encoded HMAC-SHA256 | +| `X-Lark-Proxy-Timestamp` | Unix seconds (drift ≤ 60s) | +| `X-Lark-Body-SHA256` | hex-encoded SHA-256 of the request body | + +Signing material (newline-separated, in order): + +```text +version +method +host +pathAndQuery +bodySHA256 +timestamp +identity +authHeader +``` + +Every field above is part of the canonical string. In particular, `identity` +and `authHeader` are covered so a captured request cannot be replayed with +its identity flipped (bot↔user) or its auth-header redirected (e.g. into +`Cookie`) inside the 60s drift window. + +## Source layout + +| File | Purpose | +| --- | --- | +| `main.go` | Entry point: flag parsing, server lifecycle | +| `handler.go` | `proxyHandler.ServeHTTP` — main request flow | +| `forward.go` | Forwarding HTTP client + proxy-header filter | +| `allowlist.go` | Target host / identity allowlists | +| `audit.go` | Log path/error sanitization | +| `handler_test.go` | Unit tests for all of the above | diff --git a/sidecar/server-demo/allowlist.go b/sidecar/server-demo/allowlist.go new file mode 100644 index 000000000..d37e855b4 --- /dev/null +++ b/sidecar/server-demo/allowlist.go @@ -0,0 +1,44 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar_demo + +package main + +import ( + "strings" + + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/sidecar" +) + +// buildAllowedHosts extracts the set of allowed target hostnames from +// multiple brand endpoints so the sidecar can serve both feishu and lark clients. +func buildAllowedHosts(endpoints ...core.Endpoints) map[string]bool { + hosts := make(map[string]bool) + for _, ep := range endpoints { + for _, u := range []string{ep.Open, ep.Accounts, ep.MCP} { + if idx := strings.Index(u, "://"); idx >= 0 { + hosts[u[idx+3:]] = true + } + } + } + return hosts +} + +// buildAllowedIdentities returns the set of identities the sidecar is allowed to serve, +// based on the trusted-side strict mode / SupportedIdentities configuration. +func buildAllowedIdentities(cfg *core.CliConfig) map[string]bool { + ids := make(map[string]bool) + switch { + case cfg.SupportedIdentities == 0: // unknown/unset → allow both + ids[sidecar.IdentityUser] = true + ids[sidecar.IdentityBot] = true + case cfg.SupportedIdentities&1 != 0: // SupportsUser bit + ids[sidecar.IdentityUser] = true + } + if cfg.SupportedIdentities == 0 || cfg.SupportedIdentities&2 != 0 { // SupportsBot bit + ids[sidecar.IdentityBot] = true + } + return ids +} diff --git a/sidecar/server-demo/audit.go b/sidecar/server-demo/audit.go new file mode 100644 index 000000000..6ed03b4de --- /dev/null +++ b/sidecar/server-demo/audit.go @@ -0,0 +1,51 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar_demo + +package main + +import "strings" + +// sanitizePath strips query parameters and replaces ID-like path segments +// with ":id" to prevent document tokens, chat IDs, etc. from leaking into logs. +// Example: /open-apis/docx/v1/documents/doxcnXXXX/blocks → /open-apis/docx/v1/documents/:id/blocks +func sanitizePath(pathAndQuery string) string { + // Strip query + path := pathAndQuery + if i := strings.IndexByte(path, '?'); i >= 0 { + path = path[:i] + } + // Replace ID-like segments (8+ chars, not a pure API keyword) + parts := strings.Split(path, "/") + for i, p := range parts { + if looksLikeID(p) { + parts[i] = ":id" + } + } + return strings.Join(parts, "/") +} + +// looksLikeID returns true if a path segment appears to be a resource identifier +// rather than an API route keyword. Heuristic: 8+ chars and contains a digit. +func looksLikeID(seg string) bool { + if len(seg) < 8 { + return false + } + for _, c := range seg { + if c >= '0' && c <= '9' { + return true + } + } + return false +} + +// sanitizeError returns a safe error string for logging, capped at 200 bytes +// to avoid dumping upstream response bodies into audit logs. +func sanitizeError(err error) string { + s := err.Error() + if len(s) > 200 { + return s[:200] + "..." + } + return s +} diff --git a/sidecar/server-demo/forward.go b/sidecar/server-demo/forward.go new file mode 100644 index 000000000..6cb0b0109 --- /dev/null +++ b/sidecar/server-demo/forward.go @@ -0,0 +1,51 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar_demo + +package main + +import ( + "fmt" + "net/http" + "time" + + "github.com/larksuite/cli/sidecar" +) + +// newForwardClient creates an HTTP client for forwarding requests to the +// Lark API. It strips Authorization on cross-host redirects and disables +// proxy to prevent real tokens from leaking through environment proxies. +func newForwardClient() *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil // never proxy the trusted hop + return &http.Client{ + Transport: transport, + Timeout: 30 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("too many redirects") + } + if len(via) > 0 && req.URL.Host != via[0].URL.Host { + req.Header.Del("Authorization") + req.Header.Del(sidecar.HeaderMCPUAT) + req.Header.Del(sidecar.HeaderMCPTAT) + } + return nil + }, + } +} + +// isProxyHeader returns true for headers specific to the sidecar protocol. +func isProxyHeader(key string) bool { + switch http.CanonicalHeaderKey(key) { + case 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 +} diff --git a/sidecar/server-demo/handler.go b/sidecar/server-demo/handler.go new file mode 100644 index 000000000..2fad340df --- /dev/null +++ b/sidecar/server-demo/handler.go @@ -0,0 +1,271 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar_demo + +package main + +import ( + "bytes" + "fmt" + "io" + "log" + "net/http" + "net/url" + "time" + + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/sidecar" +) + +// proxyHandler handles HTTP requests from sandbox CLI instances. +type proxyHandler struct { + key []byte + cred *credential.CredentialProvider + appID string + brand core.LarkBrand + logger *log.Logger + forwardCl *http.Client + allowedHosts map[string]bool // target host allowlist derived from brand + allowedIDs map[string]bool // identity allowlist derived from strict mode +} + +// allowedAuthHeaders lists the only header names the sidecar will inject real +// tokens into. Limiting this prevents a compromised sandbox from signing a +// request with X-Lark-Proxy-Auth-Header: Cookie (or User-Agent / +// X-Forwarded-For / any X-* header) and having the real token smuggled into +// an upstream header that Lark ignores for auth but intermediate logs may +// capture — an indirect exfiltration path. +// +// These three are the only values the CLI interceptor ever emits +// (Authorization for OpenAPI, MCP-UAT/TAT for the MCP protocol), so anything +// else is by definition a misuse. +var allowedAuthHeaders = map[string]bool{ + "Authorization": true, + sidecar.HeaderMCPUAT: true, // X-Lark-MCP-UAT + sidecar.HeaderMCPTAT: true, // X-Lark-MCP-TAT +} + +func (h *proxyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + start := time.Now() + + // 0. Check protocol version. We reject rather than default so that an + // old client paired with a newer server (or vice versa) fails loudly + // instead of silently producing mismatched signatures. + version := r.Header.Get(sidecar.HeaderProxyVersion) + if version != sidecar.ProtocolV1 { + http.Error(w, "unsupported "+sidecar.HeaderProxyVersion+": "+version, http.StatusBadRequest) + return + } + + // 1. Verify timestamp + ts := r.Header.Get(sidecar.HeaderProxyTimestamp) + if ts == "" { + http.Error(w, "missing "+sidecar.HeaderProxyTimestamp, http.StatusBadRequest) + return + } + + // 2. Read body and verify SHA256 + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "failed to read request body", http.StatusBadRequest) + return + } + r.Body.Close() + + claimedSHA := r.Header.Get(sidecar.HeaderBodySHA256) + if claimedSHA == "" { + http.Error(w, "missing "+sidecar.HeaderBodySHA256, http.StatusBadRequest) + return + } + actualSHA := sidecar.BodySHA256(body) + if claimedSHA != actualSHA { + http.Error(w, "body SHA256 mismatch", http.StatusBadRequest) + return + } + + // 3. Verify HMAC signature + //Enforce scheme=https and reject any path/query embedded in the target. + // The sandbox is untrusted: without this check it could send + // X-Lark-Proxy-Target: http://open.feishu.cn to force the injected real + // token out over cleartext HTTP, exposing it to any on-path attacker + // between the sidecar and upstream. + target := r.Header.Get(sidecar.HeaderProxyTarget) + if target == "" { + http.Error(w, "missing "+sidecar.HeaderProxyTarget, http.StatusBadRequest) + return + } + + pathAndQuery := r.URL.RequestURI() + targetHost, err := parseTarget(target) + if err != nil { + http.Error(w, "invalid "+sidecar.HeaderProxyTarget+": "+err.Error(), http.StatusForbidden) + h.logger.Printf("REJECT method=%s path=%s reason=%q", r.Method, sanitizePath(pathAndQuery), sanitizeError(err)) + return + } + + // Identity and auth-header must be read before HMAC verification because + // both are covered by the canonical signing string. Defaulting either one + // server-side would let an attacker flip the injected token's identity or + // target header within the replay window without invalidating the sig. + identity := r.Header.Get(sidecar.HeaderProxyIdentity) + if identity == "" { + http.Error(w, "missing "+sidecar.HeaderProxyIdentity, http.StatusBadRequest) + return + } + authHeader := r.Header.Get(sidecar.HeaderProxyAuthHeader) + if authHeader == "" { + http.Error(w, "missing "+sidecar.HeaderProxyAuthHeader, http.StatusBadRequest) + return + } + + signature := r.Header.Get(sidecar.HeaderProxySignature) + if err := sidecar.Verify(h.key, sidecar.CanonicalRequest{ + Version: version, + Method: r.Method, + Host: targetHost, + PathAndQuery: pathAndQuery, + BodySHA256: claimedSHA, + Timestamp: ts, + Identity: identity, + AuthHeader: authHeader, + }, signature); err != nil { + http.Error(w, "HMAC verification failed: "+err.Error(), http.StatusUnauthorized) + h.logger.Printf("REJECT method=%s path=%s reason=%q", r.Method, sanitizePath(pathAndQuery), sanitizeError(err)) + return + } + + // 4. Validate target host against allowlist + if !h.allowedHosts[targetHost] { + http.Error(w, "target host not allowed: "+targetHost, http.StatusForbidden) + h.logger.Printf("REJECT method=%s path=%s reason=\"target host %s not in allowlist\"", r.Method, sanitizePath(pathAndQuery), targetHost) + return + } + + // 5. Validate identity + if !h.allowedIDs[identity] { + http.Error(w, "identity not allowed: "+identity, http.StatusForbidden) + h.logger.Printf("REJECT method=%s path=%s reason=\"identity %s not allowed by strict mode\"", r.Method, sanitizePath(pathAndQuery), identity) + return + } + + // 5.5 Validate auth-header (required — the client controls this value, + // and without an allowlist a compromised sandbox could direct the real + // token into arbitrary forwarded headers). + if !allowedAuthHeaders[authHeader] { + http.Error(w, "auth-header not allowed: "+authHeader, http.StatusForbidden) + h.logger.Printf("REJECT method=%s path=%s reason=\"auth-header %s not in allowlist\"", r.Method, sanitizePath(pathAndQuery), authHeader) + return + } + + // 6. Resolve real token + var tokenType credential.TokenType + switch identity { + case sidecar.IdentityUser: + tokenType = credential.TokenTypeUAT + default: + tokenType = credential.TokenTypeTAT + } + + tokenResult, err := h.cred.ResolveToken(r.Context(), credential.TokenSpec{ + Type: tokenType, + AppID: h.appID, + }) + if err != nil { + http.Error(w, "failed to resolve token: "+err.Error(), http.StatusInternalServerError) + h.logger.Printf("TOKEN_ERROR method=%s path=%s identity=%s error=%q", r.Method, sanitizePath(pathAndQuery), identity, sanitizeError(err)) + return + } + + // 7. Build forwarding request. Scheme is pinned to https here (not taken + // from the client-supplied target) so any future change to parseTarget + // cannot regress the cleartext-leak protection. + forwardURL := "https://" + targetHost + pathAndQuery + forwardReq, err := http.NewRequestWithContext(r.Context(), r.Method, forwardURL, bytes.NewReader(body)) + if err != nil { + http.Error(w, "failed to create forward request", http.StatusInternalServerError) + return + } + + // Copy non-proxy headers + for k, vs := range r.Header { + if isProxyHeader(k) { + continue + } + for _, v := range vs { + forwardReq.Header.Add(k, v) + } + } + + // Strip any client-supplied auth headers. The sidecar is the sole source + // of authentication material on the forwarded request; a client could + // otherwise smuggle an extra Authorization/MCP token alongside the one + // the sidecar injects below. + forwardReq.Header.Del("Authorization") + forwardReq.Header.Del(sidecar.HeaderMCPUAT) + forwardReq.Header.Del(sidecar.HeaderMCPTAT) + + // 8. Inject real token into the header the client committed to in the + // signature. Standard OpenAPI uses "Authorization: Bearer "; MCP + // uses "X-Lark-MCP-UAT: " or "X-Lark-MCP-TAT: ". + if authHeader == "Authorization" { + forwardReq.Header.Set("Authorization", "Bearer "+tokenResult.Token) + } else { + forwardReq.Header.Set(authHeader, tokenResult.Token) + } + + // 9. Forward request + resp, err := h.forwardCl.Do(forwardReq) + if err != nil { + http.Error(w, "forward request failed: "+err.Error(), http.StatusBadGateway) + h.logger.Printf("FORWARD_ERROR method=%s path=%s error=%q", r.Method, sanitizePath(pathAndQuery), sanitizeError(err)) + return + } + defer resp.Body.Close() + + // 10. Copy response back + for k, vs := range resp.Header { + for _, v := range vs { + w.Header().Add(k, v) + } + } + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) + + // 11. Audit log + h.logger.Printf("FORWARD method=%s path=%s identity=%s status=%d duration=%s", + r.Method, sanitizePath(pathAndQuery), identity, resp.StatusCode, time.Since(start).Round(time.Millisecond)) +} + +// parseTarget validates X-Lark-Proxy-Target and returns the host portion for +// HMAC input and allowlist lookup. The target must be "https://" with no +// path, query, fragment, userinfo, or non-https scheme. Rejecting these shapes +// closes a token-leak channel: a compromised sandbox holding PROXY_KEY could +// otherwise request cleartext HTTP forwarding (or inject a path to a different +// endpoint than the allowlist entry implies). +func parseTarget(target string) (host string, err error) { + u, perr := url.Parse(target) + if perr != nil { + return "", fmt.Errorf("parse: %w", perr) + } + 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 +} diff --git a/sidecar/server-demo/handler_test.go b/sidecar/server-demo/handler_test.go new file mode 100644 index 000000000..2bc2a20f2 --- /dev/null +++ b/sidecar/server-demo/handler_test.go @@ -0,0 +1,670 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar_demo + +package main + +import ( + "bytes" + "context" + "fmt" + "io" + "log" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + extcred "github.com/larksuite/cli/extension/credential" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/sidecar" +) + +// fakeExtProvider is a stub extcred.Provider for tests that returns a fixed token. +type fakeExtProvider struct { + token string +} + +func (f *fakeExtProvider) Name() string { return "fake" } +func (f *fakeExtProvider) ResolveAccount(ctx context.Context) (*extcred.Account, error) { + return nil, nil +} +func (f *fakeExtProvider) ResolveToken(ctx context.Context, req extcred.TokenSpec) (*extcred.Token, error) { + return &extcred.Token{Value: f.token, Source: "fake"}, nil +} + +func discardLogger() *log.Logger { + return log.New(io.Discard, "", 0) +} + +func newTestHandler(key []byte) *proxyHandler { + return &proxyHandler{ + key: key, + logger: discardLogger(), + forwardCl: &http.Client{}, + allowedHosts: map[string]bool{ + "open.feishu.cn": true, + "accounts.feishu.cn": true, + "mcp.feishu.cn": true, + }, + allowedIDs: map[string]bool{ + sidecar.IdentityUser: true, + sidecar.IdentityBot: true, + }, + } +} + +// signedReq creates a properly signed request for testing handler logic past +// HMAC verification. Identity defaults to bot and auth-header to +// "Authorization"; callers can override by mutating the returned request +// before calling ServeHTTP (and re-signing if they need the signature to +// remain valid after the mutation). +func signedReq(t *testing.T, key []byte, method, target, path string, body []byte) *http.Request { + t.Helper() + targetHost := target + if idx := strings.Index(target, "://"); idx >= 0 { + targetHost = target[idx+3:] + } + bodySHA := sidecar.BodySHA256(body) + ts := sidecar.Timestamp() + identity := sidecar.IdentityBot + authHeader := "Authorization" + sig := sidecar.Sign(key, sidecar.CanonicalRequest{ + Version: sidecar.ProtocolV1, + Method: method, + Host: targetHost, + PathAndQuery: path, + BodySHA256: bodySHA, + Timestamp: ts, + Identity: identity, + AuthHeader: authHeader, + }) + + var bodyReader io.Reader + if body != nil { + bodyReader = bytes.NewReader(body) + } + req := httptest.NewRequest(method, path, bodyReader) + req.Header.Set(sidecar.HeaderProxyVersion, sidecar.ProtocolV1) + req.Header.Set(sidecar.HeaderProxyTarget, target) + req.Header.Set(sidecar.HeaderProxyIdentity, identity) + req.Header.Set(sidecar.HeaderProxyAuthHeader, authHeader) + req.Header.Set(sidecar.HeaderBodySHA256, bodySHA) + req.Header.Set(sidecar.HeaderProxyTimestamp, ts) + req.Header.Set(sidecar.HeaderProxySignature, sig) + return req +} + +// resign recomputes the HMAC signature over the request's current proxy +// headers. Use this in tests that mutate a signed field (Identity, +// AuthHeader, Target host, etc.) after calling signedReq. +func resign(t *testing.T, key []byte, req *http.Request, body []byte) { + t.Helper() + target := req.Header.Get(sidecar.HeaderProxyTarget) + targetHost := target + if idx := strings.Index(target, "://"); idx >= 0 { + targetHost = target[idx+3:] + } + sig := sidecar.Sign(key, sidecar.CanonicalRequest{ + Version: req.Header.Get(sidecar.HeaderProxyVersion), + Method: req.Method, + Host: targetHost, + PathAndQuery: req.URL.RequestURI(), + BodySHA256: sidecar.BodySHA256(body), + Timestamp: req.Header.Get(sidecar.HeaderProxyTimestamp), + Identity: req.Header.Get(sidecar.HeaderProxyIdentity), + AuthHeader: req.Header.Get(sidecar.HeaderProxyAuthHeader), + }) + req.Header.Set(sidecar.HeaderProxySignature, sig) +} + +// TestProxyHandler_UnsupportedVersion verifies the handler rejects requests +// whose HeaderProxyVersion is absent or set to an unknown value. Kept in +// front so an old client paired with a newer server (or vice versa) surfaces +// a clear 400 instead of a misleading HMAC mismatch downstream. +func TestProxyHandler_UnsupportedVersion(t *testing.T) { + h := newTestHandler([]byte("key")) + for _, v := range []string{"", "v0", "v2"} { + req := httptest.NewRequest("GET", "/path", nil) + if v != "" { + req.Header.Set(sidecar.HeaderProxyVersion, v) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("version=%q: expected 400, got %d", v, w.Code) + } + } +} + +func TestProxyHandler_MissingTimestamp(t *testing.T) { + h := newTestHandler([]byte("key")) + req := httptest.NewRequest("GET", "/path", nil) + req.Header.Set(sidecar.HeaderProxyVersion, sidecar.ProtocolV1) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400, got %d", w.Code) + } +} + +func TestProxyHandler_MissingBodySHA(t *testing.T) { + h := newTestHandler([]byte("key")) + req := httptest.NewRequest("GET", "/path", nil) + req.Header.Set(sidecar.HeaderProxyVersion, sidecar.ProtocolV1) + req.Header.Set(sidecar.HeaderProxyTimestamp, sidecar.Timestamp()) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400, got %d", w.Code) + } +} + +func TestProxyHandler_BadHMAC(t *testing.T) { + h := newTestHandler([]byte("real-key")) + + bodySHA := sidecar.BodySHA256(nil) + ts := sidecar.Timestamp() + + req := httptest.NewRequest("GET", "/path", nil) + req.Header.Set(sidecar.HeaderProxyVersion, sidecar.ProtocolV1) + req.Header.Set(sidecar.HeaderProxyTarget, "https://open.feishu.cn") + req.Header.Set(sidecar.HeaderProxyIdentity, sidecar.IdentityBot) + req.Header.Set(sidecar.HeaderProxyAuthHeader, "Authorization") + req.Header.Set(sidecar.HeaderProxyTimestamp, ts) + req.Header.Set(sidecar.HeaderBodySHA256, bodySHA) + req.Header.Set(sidecar.HeaderProxySignature, "bad-signature") + + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestProxyHandler_BodySHA256Mismatch(t *testing.T) { + h := newTestHandler([]byte("key")) + + req := httptest.NewRequest("POST", "/path", bytes.NewReader([]byte("real body"))) + req.Header.Set(sidecar.HeaderProxyVersion, sidecar.ProtocolV1) + req.Header.Set(sidecar.HeaderProxyTarget, "https://open.feishu.cn") + req.Header.Set(sidecar.HeaderProxyTimestamp, sidecar.Timestamp()) + req.Header.Set(sidecar.HeaderBodySHA256, sidecar.BodySHA256([]byte("different body"))) + req.Header.Set(sidecar.HeaderProxySignature, "whatever") + + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400, got %d", w.Code) + } +} + +func TestProxyHandler_TargetNotAllowed(t *testing.T) { + key := []byte("test-key") + h := newTestHandler(key) + + req := signedReq(t, key, "GET", "https://evil.com", "/steal", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for disallowed host, got %d", w.Code) + } +} + +func TestProxyHandler_IdentityNotAllowed(t *testing.T) { + key := []byte("test-key") + h := newTestHandler(key) + // Restrict to bot only + h.allowedIDs = map[string]bool{sidecar.IdentityBot: true} + + req := signedReq(t, key, "GET", "https://open.feishu.cn", "/open-apis/test", nil) + req.Header.Set(sidecar.HeaderProxyIdentity, sidecar.IdentityUser) + resign(t, key, req, nil) // identity is signed; must re-sign after mutation + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for disallowed identity, got %d", w.Code) + } +} + +// TestParseTarget covers the per-shape rejections directly, without the +// surrounding HTTP plumbing. +func TestParseTarget(t *testing.T) { + cases := []struct { + name string + target string + wantErr bool + wantSub string // expected fragment of the error message + }{ + {name: "valid https", target: "https://open.feishu.cn", wantErr: false}, + {name: "valid https trailing slash", target: "https://open.feishu.cn/", wantErr: false}, + {name: "http downgrade", target: "http://open.feishu.cn", wantErr: true, wantSub: "scheme must be https"}, + {name: "missing scheme", target: "open.feishu.cn", wantErr: true, wantSub: "scheme must be https"}, + {name: "ftp scheme", target: "ftp://open.feishu.cn", wantErr: true, wantSub: "scheme must be https"}, + {name: "empty", target: "", wantErr: true, wantSub: "scheme must be https"}, + {name: "empty host", target: "https://", wantErr: true, wantSub: "missing host"}, + {name: "with path", target: "https://open.feishu.cn/open-apis", wantErr: true, wantSub: "path not allowed"}, + {name: "with query", target: "https://open.feishu.cn?a=1", wantErr: true, wantSub: "query not allowed"}, + {name: "with fragment", target: "https://open.feishu.cn#frag", wantErr: true, wantSub: "fragment not allowed"}, + {name: "with userinfo", target: "https://attacker:pw@open.feishu.cn", wantErr: true, wantSub: "userinfo not allowed"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + host, err := parseTarget(tc.target) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got host=%q", host) + } + if tc.wantSub != "" && !strings.Contains(err.Error(), tc.wantSub) { + t.Errorf("error %q should contain %q", err.Error(), tc.wantSub) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if host != "open.feishu.cn" { + t.Errorf("host = %q, want %q", host, "open.feishu.cn") + } + }) + } +} + +// TestProxyHandler_RejectsNonHTTPSTarget verifies end-to-end that a +// compromised sandbox holding a valid PROXY_KEY cannot coerce the sidecar +// into forwarding real tokens over cleartext HTTP or to an unexpected path. +// The check must fire before HMAC verification so that the request is +// rejected even when the signature is technically valid. +func TestProxyHandler_RejectsNonHTTPSTarget(t *testing.T) { + key := []byte("test-key") + h := newTestHandler(key) + + cases := []struct { + name string + target string + }{ + {"http downgrade", "http://open.feishu.cn"}, + {"bare hostname", "open.feishu.cn"}, + {"ftp scheme", "ftp://open.feishu.cn"}, + {"target with path", "https://open.feishu.cn/open-apis/evil"}, + {"target with query", "https://open.feishu.cn?steal=1"}, + {"target with userinfo", "https://attacker:pw@open.feishu.cn"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Sign with a valid key against the malicious target — proves the + // scheme/shape check is not bypassed by signature legitimacy. + req := signedReq(t, key, "GET", tc.target, "/open-apis/im/v1/chats", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for target %q, got %d (body: %s)", tc.target, w.Code, w.Body.String()) + } + }) + } +} + +// TestProxyHandler_RejectsIdentityReplay locks in C1 end-to-end: a captured +// bot-signed request whose identity header is flipped to user (or vice versa) +// must be rejected at HMAC verification, not silently served with the wrong +// token type. Without identity in the canonical string this returns 200. +func TestProxyHandler_RejectsIdentityReplay(t *testing.T) { + key := []byte("test-key") + h := newTestHandler(key) + + req := signedReq(t, key, "GET", "https://open.feishu.cn", "/open-apis/test", nil) + // Attacker flips identity without touching signature. + req.Header.Set(sidecar.HeaderProxyIdentity, sidecar.IdentityUser) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Errorf("identity replay must fail signature verify (got %d, want 401): %s", + w.Code, w.Body.String()) + } +} + +// TestProxyHandler_RejectsAuthHeaderReplay is the companion: flipping +// X-Lark-Proxy-Auth-Header post-signature must invalidate the signature so +// an attacker cannot redirect the injected token into an unintended header. +func TestProxyHandler_RejectsAuthHeaderReplay(t *testing.T) { + key := []byte("test-key") + h := newTestHandler(key) + + req := signedReq(t, key, "GET", "https://open.feishu.cn", "/open-apis/test", nil) + req.Header.Set(sidecar.HeaderProxyAuthHeader, "Cookie") + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Errorf("auth-header replay must fail signature verify (got %d, want 401): %s", + w.Code, w.Body.String()) + } +} + +// TestProxyHandler_RejectsAuthHeaderNotInAllowlist pins the auth-header +// allowlist: even a correctly-signed request must be rejected if it asks +// the sidecar to inject the real token into an unintended header (e.g. +// Cookie / User-Agent / X-Forwarded-For). This closes the sidechannel +// where the real token ends up in headers that Lark ignores for auth but +// intermediate logs may capture. +func TestProxyHandler_RejectsAuthHeaderNotInAllowlist(t *testing.T) { + key := []byte("test-key") + h := newTestHandler(key) + + for _, bad := range []string{"Cookie", "User-Agent", "X-Forwarded-For", "X-Real-IP", "Set-Cookie"} { + t.Run(bad, func(t *testing.T) { + req := signedReq(t, key, "GET", "https://open.feishu.cn", "/open-apis/test", nil) + req.Header.Set(sidecar.HeaderProxyAuthHeader, bad) + resign(t, key, req, nil) // auth-header is signed; must re-sign after override + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Errorf("authHeader=%q: expected 403, got %d (body: %s)", + bad, w.Code, w.Body.String()) + } + }) + } +} + +// TestProxyHandler_AcceptsAllowedAuthHeaders confirms the three protocol +// header names remain accepted after the allowlist is enforced. Uses +// newTestHandler which has no upstream forwarding set up, so reaching the +// forward step is proof the auth-header check passed. +func TestProxyHandler_AcceptsAllowedAuthHeaders(t *testing.T) { + key := []byte("test-key") + + for _, good := range []string{"Authorization", sidecar.HeaderMCPUAT, sidecar.HeaderMCPTAT} { + t.Run(good, func(t *testing.T) { + // Use a handler with a real (fake) credential provider so we can + // distinguish auth-header reject (403) from later failures. + cred := credential.NewCredentialProvider( + []extcred.Provider{&fakeExtProvider{token: "real-token"}}, + nil, nil, nil, + ) + h := &proxyHandler{ + key: key, + cred: cred, + appID: "cli_test", + logger: discardLogger(), + forwardCl: &http.Client{}, + allowedHosts: map[string]bool{"open.feishu.cn": true}, + allowedIDs: map[string]bool{sidecar.IdentityUser: true, sidecar.IdentityBot: true}, + } + + req := signedReq(t, key, "GET", "https://open.feishu.cn", "/open-apis/test", nil) + req.Header.Set(sidecar.HeaderProxyAuthHeader, good) + resign(t, key, req, nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + // Expect NOT 403 "auth-header not allowed" — the request will fail + // at forward (502 because open.feishu.cn isn't reachable without + // an actual upstream in tests), but it must get past our check. + if w.Code == http.StatusForbidden && strings.Contains(w.Body.String(), "auth-header not allowed") { + t.Errorf("authHeader=%q was rejected by allowlist: %s", good, w.Body.String()) + } + }) + } +} + +func TestRun_RejectsSelfProxy(t *testing.T) { + old, had := os.LookupEnv(envvars.CliAuthProxy) + os.Setenv(envvars.CliAuthProxy, "http://127.0.0.1:16384") + defer func() { + if had { + os.Setenv(envvars.CliAuthProxy, old) + } else { + os.Unsetenv(envvars.CliAuthProxy) + } + }() + + err := run(context.Background(), "127.0.0.1:0", "/tmp/should-not-be-created.key", "", "") + if err == nil { + t.Fatal("expected error when AUTH_PROXY is set") + } + if !strings.Contains(err.Error(), envvars.CliAuthProxy) { + t.Errorf("error should mention %s, got: %v", envvars.CliAuthProxy, err) + } +} + +func TestForwardClient_RedirectStripsAuth(t *testing.T) { + redirectTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if auth := r.Header.Get("Authorization"); auth != "" { + t.Errorf("Authorization leaked to redirect target: %s", auth) + } + w.WriteHeader(http.StatusOK) + })) + defer redirectTarget.Close() + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, redirectTarget.URL+"/redirected", http.StatusFound) + })) + defer origin.Close() + + client := newForwardClient() + req, _ := http.NewRequest("GET", origin.URL+"/start", nil) + req.Header.Set("Authorization", "Bearer real-token") + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() +} + +func TestForwardClient_RedirectStripsMCPHeaders(t *testing.T) { + redirectTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if v := r.Header.Get(sidecar.HeaderMCPUAT); v != "" { + t.Errorf("X-Lark-MCP-UAT leaked to redirect target: %s", v) + } + if v := r.Header.Get(sidecar.HeaderMCPTAT); v != "" { + t.Errorf("X-Lark-MCP-TAT leaked to redirect target: %s", v) + } + w.WriteHeader(http.StatusOK) + })) + defer redirectTarget.Close() + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, redirectTarget.URL+"/redirected", http.StatusFound) + })) + defer origin.Close() + + client := newForwardClient() + req, _ := http.NewRequest("POST", origin.URL+"/mcp", nil) + req.Header.Set(sidecar.HeaderMCPUAT, "real-uat-token") + req.Header.Set(sidecar.HeaderMCPTAT, "real-tat-token") + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() +} + +// TestProxyHandler_StripsClientSuppliedAuthHeaders verifies that the sidecar +// is the sole source of auth headers on the forwarded request. A malicious +// sandbox client must not be able to smuggle an Authorization/MCP header that +// rides along with the sidecar-injected real token. +func TestProxyHandler_StripsClientSuppliedAuthHeaders(t *testing.T) { + const realToken = "real-tenant-access-token" + + // Capture what the upstream receives after sidecar forwarding. + // TLS is required because parseTarget rejects non-https targets. + var captured http.Header + upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured = r.Header.Clone() + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + + // Strip "https://" prefix to get host:port (matches what the handler sees). + upstreamHost := strings.TrimPrefix(upstream.URL, "https://") + + cred := credential.NewCredentialProvider( + []extcred.Provider{&fakeExtProvider{token: realToken}}, + nil, nil, nil, + ) + + key := []byte("test-key") + h := &proxyHandler{ + key: key, + cred: cred, + appID: "cli_test", + logger: discardLogger(), + forwardCl: upstream.Client(), // trusts the httptest CA + allowedHosts: map[string]bool{upstreamHost: true}, + allowedIDs: map[string]bool{sidecar.IdentityUser: true, sidecar.IdentityBot: true}, + } + + cases := []struct { + name string + proxyAuthHeader string // which header sidecar should inject into + wantInjectedHeader string // the header the real token ends up in + wantInjectedValue string + wantStrippedHeaders []string + }{ + { + name: "inject Authorization, strip MCP attacker headers", + proxyAuthHeader: "Authorization", + wantInjectedHeader: "Authorization", + wantInjectedValue: "Bearer " + realToken, + wantStrippedHeaders: []string{sidecar.HeaderMCPUAT, sidecar.HeaderMCPTAT}, + }, + { + name: "inject MCP UAT, strip Authorization attacker header", + proxyAuthHeader: sidecar.HeaderMCPUAT, + wantInjectedHeader: sidecar.HeaderMCPUAT, + wantInjectedValue: realToken, + wantStrippedHeaders: []string{"Authorization", sidecar.HeaderMCPTAT}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + captured = nil + + req := signedReq(t, key, "GET", "https://"+upstreamHost, "/open-apis/test", nil) + req.Header.Set(sidecar.HeaderProxyAuthHeader, tc.proxyAuthHeader) + resign(t, key, req, nil) // auth-header is signed; re-sign after override + + // Attacker smuggles all three possible auth headers with bogus values. + req.Header.Set("Authorization", "Bearer attacker-token") + req.Header.Set(sidecar.HeaderMCPUAT, "attacker-uat") + req.Header.Set(sidecar.HeaderMCPTAT, "attacker-tat") + + // Non-auth headers should still pass through. + req.Header.Set("X-Custom-Header", "keep-me") + + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200 from upstream, got %d; body=%s", w.Code, w.Body.String()) + } + if captured == nil { + t.Fatal("upstream handler was not invoked") + } + + // Injected header contains the real token (not the attacker value). + if got := captured.Get(tc.wantInjectedHeader); got != tc.wantInjectedValue { + t.Errorf("%s = %q, want %q", tc.wantInjectedHeader, got, tc.wantInjectedValue) + } + + // All other auth headers must be stripped. + for _, h := range tc.wantStrippedHeaders { + if got := captured.Get(h); got != "" { + t.Errorf("%s should be stripped, got %q", h, got) + } + } + + // Non-auth headers still forwarded. + if got := captured.Get("X-Custom-Header"); got != "keep-me" { + t.Errorf("X-Custom-Header = %q, want %q", got, "keep-me") + } + }) + } +} + +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", + } + hosts := buildAllowedHosts(feishu, lark) + // feishu hosts + if !hosts["open.feishu.cn"] { + t.Error("expected open.feishu.cn in allowlist") + } + if !hosts["mcp.feishu.cn"] { + t.Error("expected mcp.feishu.cn in allowlist") + } + // lark hosts + if !hosts["open.larksuite.com"] { + t.Error("expected open.larksuite.com in allowlist") + } + if !hosts["mcp.larksuite.com"] { + t.Error("expected mcp.larksuite.com in allowlist") + } + // evil host + if hosts["evil.com"] { + t.Error("evil.com should not be in allowlist") + } +} + +func TestSanitizePath(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"/open-apis/im/v1/messages?receive_id_type=chat_id", "/open-apis/im/v1/messages"}, + {"/open-apis/calendar/v4/events", "/open-apis/calendar/v4/events"}, + {"/open-apis/docx/v1/documents/doxcnABCD1234/blocks", "/open-apis/docx/v1/documents/:id/blocks"}, + {"/open-apis/im/v1/chats/oc_abcdef12345678/members", "/open-apis/im/v1/chats/:id/members"}, + {"/path?secret=abc", "/path"}, + } + for _, tt := range tests { + if got := sanitizePath(tt.input); got != tt.want { + t.Errorf("sanitizePath(%q) = %q, want %q", tt.input, got, tt.want) + } + } +} + +func TestLooksLikeID(t *testing.T) { + tests := []struct { + seg string + want bool + }{ + {"doxcnABCD1234", true}, // doc token + {"oc_abcdef12345678", true}, // chat ID + {"v1", false}, // API version + {"messages", false}, // route keyword + {"open-apis", false}, // route prefix + {"ab1", false}, // too short + } + for _, tt := range tests { + if got := looksLikeID(tt.seg); got != tt.want { + t.Errorf("looksLikeID(%q) = %v, want %v", tt.seg, got, tt.want) + } + } +} + +func TestSanitizeError(t *testing.T) { + short := fmt.Errorf("short error") + if got := sanitizeError(short); got != "short error" { + t.Errorf("got %q", got) + } + + longMsg := make([]byte, 300) + for i := range longMsg { + longMsg[i] = 'x' + } + long := fmt.Errorf("%s", string(longMsg)) + got := sanitizeError(long) + if len(got) > 210 { + t.Errorf("expected truncation, got %d chars", len(got)) + } + if !bytes.HasSuffix([]byte(got), []byte("...")) { + t.Errorf("expected '...' suffix, got %q", got[len(got)-10:]) + } +} diff --git a/sidecar/server-demo/main.go b/sidecar/server-demo/main.go new file mode 100644 index 000000000..1197b5145 --- /dev/null +++ b/sidecar/server-demo/main.go @@ -0,0 +1,167 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar_demo + +// Command sidecar-server-demo is a reference implementation of a sidecar +// auth proxy server. It is NOT production-ready — integrators should +// implement their own server conforming to the wire protocol defined in +// github.com/larksuite/cli/sidecar. +// +// The demo reuses the lark-cli credential pipeline (keychain + config) to +// resolve real tokens, so it only works on a machine that has been +// configured with `lark-cli auth login`. +package main + +import ( + "context" + "crypto/rand" + "encoding/hex" + "flag" + "fmt" + "log" + "net" + "net/http" + "os" + "os/signal" + "path/filepath" + "syscall" + "time" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/internal/vfs" + "github.com/larksuite/cli/sidecar" +) + +func main() { + listen := flag.String("listen", sidecar.DefaultListenAddr, "listen address (host:port)") + keyFile := flag.String("key-file", defaultKeyFile(), "path to write the HMAC key") + logFile := flag.String("log-file", "", "audit log file (stderr if empty)") + profile := flag.String("profile", "", "lark-cli profile name (empty = active profile)") + flag.Parse() + + ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer cancel() + + if err := run(ctx, *listen, *keyFile, *logFile, *profile); err != nil { + fmt.Fprintln(os.Stderr, "error:", err) + os.Exit(1) + } +} + +func defaultKeyFile() string { + if home, err := os.UserHomeDir(); err == nil { + return filepath.Join(home, ".lark-sidecar", "proxy.key") + } + return "/tmp/lark-sidecar/proxy.key" +} + +func run(ctx context.Context, listen, keyFile, logFile, profile string) error { + // Reject self-proxy: if this process inherited AUTH_PROXY, the sidecar + // credential provider would activate and return sentinel tokens instead + // of real ones, breaking the "trusted side holds real credentials" premise. + if v := os.Getenv(envvars.CliAuthProxy); v != "" { + return fmt.Errorf("%s is set in this environment (%s); unset it before starting the sidecar server", envvars.CliAuthProxy, v) + } + if listen == "" { + return fmt.Errorf("invalid --listen address: empty") + } + + // Generate HMAC key (32 bytes = 256 bits) and write it to disk (0600). + keyBytes := make([]byte, 32) + if _, err := rand.Read(keyBytes); err != nil { + return fmt.Errorf("failed to generate HMAC key: %v", err) + } + keyHex := hex.EncodeToString(keyBytes) + + keyDir := filepath.Dir(keyFile) + if err := vfs.MkdirAll(keyDir, 0700); err != nil { + return fmt.Errorf("failed to create key directory: %v", err) + } + if err := vfs.WriteFile(keyFile, []byte(keyHex), 0600); err != nil { + return fmt.Errorf("failed to write key file: %v", err) + } + + // Audit logger: file or stderr. + var auditLogger *log.Logger + if logFile != "" { + f, err := vfs.OpenFile(logFile, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) + if err != nil { + return fmt.Errorf("failed to open log file: %v", err) + } + defer f.Close() + auditLogger = log.New(f, "", log.LstdFlags) + } else { + auditLogger = log.New(os.Stderr, "[audit] ", log.LstdFlags) + } + + // Reuse the lark-cli credential pipeline. A production implementation + // would likely source credentials from a secrets manager instead. + factory := cmdutil.NewDefault(cmdutil.InvocationContext{Profile: profile}) + cfg, err := factory.Config() + if err != nil { + return fmt.Errorf("failed to load config: %v", err) + } + + listener, err := net.Listen("tcp", listen) + if err != nil { + return fmt.Errorf("failed to listen on %s: %v", listen, err) + } + defer listener.Close() + + allowedHosts := buildAllowedHosts( + core.ResolveEndpoints(core.BrandFeishu), + core.ResolveEndpoints(core.BrandLark), + ) + allowedIDs := buildAllowedIdentities(cfg) + + handler := &proxyHandler{ + key: []byte(keyHex), + cred: factory.Credential, + appID: cfg.AppID, + brand: cfg.Brand, + logger: auditLogger, + forwardCl: newForwardClient(), + allowedHosts: allowedHosts, + allowedIDs: allowedIDs, + } + + server := &http.Server{ + Handler: handler, + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 60 * time.Second, + IdleTimeout: 120 * time.Second, + MaxHeaderBytes: 1 << 20, + } + + go func() { + <-ctx.Done() + auditLogger.Println("shutting down...") + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := server.Shutdown(shutdownCtx); err != nil { + auditLogger.Printf("shutdown error: %v", err) + } + }() + + keyPrefix := keyHex + if len(keyPrefix) > 8 { + keyPrefix = keyPrefix[:8] + } + proxyURL := "http://" + listen + fmt.Fprintf(os.Stderr, "Auth sidecar listening on %s\n", proxyURL) + fmt.Fprintf(os.Stderr, "HMAC key prefix: %s\n", keyPrefix) + fmt.Fprintf(os.Stderr, "Full key written to %s (mode 0600)\n", keyFile) + fmt.Fprintf(os.Stderr, "\nSet in sandbox:\n") + fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliAuthProxy, proxyURL) + fmt.Fprintf(os.Stderr, " export %s=\"\"\n", envvars.CliProxyKey, keyFile) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliAppID, cfg.AppID) + fmt.Fprintf(os.Stderr, " export %s=%q\n", envvars.CliBrand, string(cfg.Brand)) + + if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { + return fmt.Errorf("sidecar server exited unexpectedly: %v", err) + } + return nil +} From 67e51ec8d74979327dd8546683ace0ff5f669ad6 Mon Sep 17 00:00:00 2001 From: zhengquanbin Date: Mon, 20 Apr 2026 21:08:04 +0800 Subject: [PATCH 4/7] fix: base role view & record default perm on edit(#530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix: address coderabbit review comments on role-config docs - Update `allow_edit` field description to reflect conditional default: `true` when table perm is `edit`, `false` for `read_only` or explicit restriction - Move `record_operations.delete` out of "默认关闭项" into new "默认开启项（条件性）" section to accurately reflect it is default-included when `perm = edit` - Add `view_rule.allow_edit` to "默认开启项（条件性）" section with same logic Co-authored-by: Claude Sonnet 4.6 (1M context) --- skills/lark-base/references/role-config.md | 24 ++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/skills/lark-base/references/role-config.md b/skills/lark-base/references/role-config.md index 9c54cc478..aad99b24d 100644 --- a/skills/lark-base/references/role-config.md +++ b/skills/lark-base/references/role-config.md @@ -193,7 +193,7 @@ | 字段 | 类型 | 说明 | |------|------|----------------------------| -| `allow_edit` | bool | 可新增、删除、修改视图，未提及默认为 `false` | +| `allow_edit` | bool | 可新增、删除、修改视图；表权限为 `edit` 时默认为 `true`，表权限为 `read_only` 或用户明确限制时为 `false` | | `visibility` | object | 可见的视图配置 | | `visibility.all_visible` | bool | 是否全部可见 | | `visibility.visible_views` | []string | 可见视图名称 列表 | @@ -203,7 +203,7 @@ 输出 `view_rule` 时，**必须**使用以下完整结构，根据场景选择对应模板： ```json -// 情况 A：用户要求可编辑/新增/删除视图 → allow_edit 设为 true +// 情况 A：表权限为 edit 且用户未明确限制 → allow_edit 默认为 true，全部可见 { "view_rule": { "allow_edit": true, @@ -213,7 +213,7 @@ } } -// 情况 B：用户未提及具体视图，未要求编辑视图 → 全部可见、不可编辑 +// 情况 B：表权限为 read_only，或用户明确说不可编辑视图 → 全部可见、不可编辑 { "view_rule": { "allow_edit": false, @@ -223,10 +223,10 @@ } } -// 情况 C：用户提及了具体视图 → 仅指定视图可见 +// 情况 C：用户提及了具体视图 → 仅指定视图可见（allow_edit 仍按 A/B 规则判断） { "view_rule": { - "allow_edit": false, + "allow_edit": true, "visibility": { "all_visible": false, "visible_views": ["表格视图", "看板视图"] @@ -415,7 +415,15 @@ | 仪表盘访问 | 不配置 | 用户明确提及该仪表盘 | | `base_rule_map.copy` | `false` | 用户明确要求"允许复制" | | `base_rule_map.download` | `false` | 用户明确要求"允许下载/打印/副本" | -| `record_operations` 中的 `delete` | 不包含 | 用户明确说"允许删除"或使用强语义（"完全管理""可删改"） | + +### 默认开启项（条件性） + +以下能力在特定条件下**默认开启**，用户明确限制时才排除： + +| 能力 | 默认值 | 排除条件 | +|------|--------|----------| +| `record_operations` 中的 `delete` | **包含**（`perm = edit` 时） | 用户明确限制时才排除 | +| `view_rule.allow_edit` | **`true`**（`perm = edit` 时） | 用户明确限制"不可编辑视图"或 `perm = read_only` 时设为 `false` | --- @@ -436,7 +444,7 @@ ### 记录操作默认策略 **注意**: -- 用户未提及时，默认包含 `add`，默认不包含 `delete` +- 用户未提及时，表权限为 `edit` 时默认同时包含 `add` 和 `delete`，默认不包含 `delete` 的情况仅适用于用户明确限制操作的场景 - 阅读范围默认对齐编辑范围：用户仅描述可编辑范围、未说明阅读范围时，可阅读范围与可编辑范围保持一致，不主动扩大 - 当可读范围与可编辑范围一致时，**不得**生成 `read_filter_rule_group`；应设置 `other_record_all_read = false` 且 `read_filter_rule_group = null` @@ -475,7 +483,7 @@ 1. **先判断用户是否提及了具体视图名称**（如"看板视图可见""甘特图不可编辑"等） - **是** → `all_visible = false`，`visible_views` 仅包含用户明确提及为"可见"的视图名称（非 viewID）；未提及的视图视为不可见 - **否**（用户完全未提及任何视图）→ `all_visible = true` -2. `allow_edit` 默认为 `false`，仅当用户明确要求"可编辑视图""可新增/删除视图""可管理视图"时才设为 `true`。设为 `true` 时仍**必须**包含 `visibility` 字段（参考视图权限 情况 A） +2. `allow_edit` 在表权限为 `edit` 时**默认为 `true`**；仅当用户明确限制"不可编辑视图"时才设为 `false`。设为 `true` 时仍**必须**包含 `visibility` 字段（参考视图权限 情况 A） 3. `all_visible` 为 `false` 时，`visible_views` **不可为空**，必须至少包含一个视图 **❌ 常见错误 — 缺少 `visibility` 字段：** From 0a0cdc8879d9fd68ec0fd7c916f3c649a34abe1f Mon Sep 17 00:00:00 2001 From: liangshuo-1 Date: Mon, 20 Apr 2026 22:03:08 +0800 Subject: [PATCH 5/7] chore(release): v1.0.15 (#575) --- CHANGELOG.md | 17 +++++++++++++++++ package.json | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01cddb4c5..5eb719b23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ All notable changes to this project will be documented in this file. +## [v1.0.15] - 2026-04-20 + +### Features + +- **sheets**: Add float image shortcuts (#494) +- **approval**: Document `remind` and `initiated` methods in skill (#554) + +### Bug Fixes + +- **base**: Preserve attachment metadata on base uploads (#563) +- **base**: Fix role view and record default permission on edit (#530) +- **sheets**: Normalize single-cell range in `+set-style` and `+batch-set-style` (#548) +- **im**: Cap `basic_batch` user_ids at 10 per API limit (#551) +- **install**: Refine install wizard messages (#529) +- **whiteboard**: Deprecate old `lark-whiteboard-cli` skill (#547) + ## [v1.0.14] - 2026-04-17 ### Features @@ -404,6 +420,7 @@ Bundled AI agent skills for intelligent assistance: - Bilingual documentation (English & Chinese). - CI/CD pipelines: linting, testing, coverage reporting, and automated releases. +[v1.0.15]: https://github.com/larksuite/cli/releases/tag/v1.0.15 [v1.0.14]: https://github.com/larksuite/cli/releases/tag/v1.0.14 [v1.0.13]: https://github.com/larksuite/cli/releases/tag/v1.0.13 [v1.0.12]: https://github.com/larksuite/cli/releases/tag/v1.0.12 diff --git a/package.json b/package.json index 15f8ca993..b7d5c903b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@larksuite/cli", - "version": "1.0.14", + "version": "1.0.15", "description": "The official CLI for Lark/Feishu open platform", "bin": { "lark-cli": "scripts/run.js" From 293a9f896f5d27cc81020861a62e521221900974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E6=98=A5=E6=99=96?= <18220699480@163.com> Date: Mon, 20 Apr 2026 22:40:56 +0800 Subject: [PATCH 6/7] fix(doc): preserve round-trip formatting in fetch output (#469) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(doc): preserve round-trip formatting in fetch output - trim leading spaces inside bold and italic emphasis exported by docs +fetch - normalize nested list indentation to avoid flattening and literal text on re-import - add regression tests for emphasis spacing and nested list indentation * fix(doc): avoid false positives in markdown spacing fixes - keep literal * x * and ** x ** text unchanged - only normalize indented nested list markers when a parent list item exists - add regression coverage for both CodeRabbit findings * fix(doc): 修正嵌套列表缩进的空行误判 - 遇到空行时停止向上查找父级列表项，避免把 loose list sibling 误改成嵌套列表 - 避免把列表项中的四空格缩进代码块误改成 tab 缩进列表项 - 补充两个回归测试，并更新 fixBoldSpacing 注释使其与当前实现一致 * fix(doc): 修复 Markdown emphasis 空格回写 - 将 fixBoldSpacingLine 改为按星号 run 扫描，修复 ** hello **、* hello * 和同一行多个 italic span 的空格清理 - 保留 inline code、heading 和 *** hello** 这类近邻字面量，避免误改 emphasis nesting --- shortcuts/doc/markdown_fix.go | 176 ++++++++++++++++++++++++----- shortcuts/doc/markdown_fix_test.go | 107 ++++++++++++++++++ 2 files changed, 257 insertions(+), 26 deletions(-) diff --git a/shortcuts/doc/markdown_fix.go b/shortcuts/doc/markdown_fix.go index f9b28dccf..1ead7a61c 100644 --- a/shortcuts/doc/markdown_fix.go +++ b/shortcuts/doc/markdown_fix.go @@ -6,6 +6,8 @@ package doc import ( "regexp" "strings" + "unicode" + "unicode/utf8" ) // fixExportedMarkdown applies post-processing to Lark-exported Markdown to @@ -15,24 +17,29 @@ import ( // and strips redundant ** from ATX headings. Applied only outside fenced // code blocks, and skips inline code spans. // -// 2. fixSetextAmbiguity: inserts a blank line before any "---" that immediately +// 2. normalizeNestedListIndentation: rewrites space-pair-indented nested list +// markers to tab-indented markers. This avoids nested ordered list items +// being flattened or interpreted as plain text/code on re-import. +// +// 3. fixSetextAmbiguity: inserts a blank line before any "---" that immediately // follows a non-empty line, preventing it from being parsed as a Setext H2. // Applied only outside fenced code blocks. // -// 3. fixBlockquoteHardBreaks: inserts a blank blockquote line (">") between +// 4. fixBlockquoteHardBreaks: inserts a blank blockquote line (">") between // consecutive blockquote content lines so create-doc preserves line breaks. // Applied only outside fenced code blocks. // -// 4. fixTopLevelSoftbreaks: inserts a blank line between adjacent non-empty +// 5. fixTopLevelSoftbreaks: inserts a blank line between adjacent non-empty // lines at the top level and inside content containers (callout, // quote-container, lark-td). Code fences are left untouched, and // consecutive list items / continuations are not separated. // -// 5. fixCalloutEmoji: replaces named emoji aliases (e.g. emoji="warning") with +// 6. fixCalloutEmoji: replaces named emoji aliases (e.g. emoji="warning") with // actual Unicode emoji characters that create-doc understands. Applied only // outside fenced code blocks. func fixExportedMarkdown(md string) string { md = applyOutsideCodeFences(md, fixBoldSpacing) + md = applyOutsideCodeFences(md, normalizeNestedListIndentation) md = applyOutsideCodeFences(md, fixSetextAmbiguity) md = applyOutsideCodeFences(md, fixBlockquoteHardBreaks) md = fixTopLevelSoftbreaks(md) @@ -106,20 +113,21 @@ func fixBlockquoteHardBreaks(md string) string { return strings.Join(out, "\n") } -// fixBoldSpacing fixes two issues with bold markers exported by Lark: +// fixBoldSpacing normalizes emphasis markers exported by Lark while preserving +// inline code spans: +// +// 1. Removes leading whitespace after opening ** and * delimiters: +// "** text**" → "**text**", "* text*" → "*text*" // -// 1. Trailing whitespace before closing **: "**text **" → "**text**" -// CommonMark requires no space before a closing delimiter; otherwise the -// ** is rendered as literal text. +// 2. Removes trailing whitespace before closing ** and * delimiters: +// "**text **" → "**text**", "*text *" → "*text*" // -// 2. Redundant bold in ATX headings: "# **text**" → "# text" -// Headings are already bold, so the inner ** is visually redundant and -// some renderers display the markers literally. +// 3. Removes redundant bold around an entire ATX heading: +// "# **text**" → "# text" // -// Both fixes skip inline code spans to avoid modifying literal code content. +// The bold and italic spacing fixes only run on non-code segments so literal +// code content is left unchanged. var ( - boldTrailingSpaceRe = regexp.MustCompile(`(\*\*\S[^*]*?)\s+(\*\*)`) - italicTrailingSpaceRe = regexp.MustCompile(`(\*\S[^*]*?)\s+(\*)`) // headingBoldRe uses [^*]+ (no asterisks) to avoid mismatching headings // that contain multiple disjoint bold spans such as "# **foo** and **bar**". headingBoldRe = regexp.MustCompile(`(?m)^(#{1,6})\s+\*\*([^*]+)\*\*\s*$`) @@ -182,38 +190,116 @@ func scanInlineCodeSpans(line string) [][2]int { // fixBoldSpacingLine applies bold/italic trailing-space fixes to a single line, // skipping content inside inline code spans to avoid corrupting literal code. // ATX heading lines are also skipped here because headingBoldRe in fixBoldSpacing -// handles them separately and boldTrailingSpaceRe can misfire on headings with -// multiple disjoint bold spans (e.g. "# **foo** and **bar**"). +// handles them separately, keeping heading-only normalization isolated from the +// inline emphasis spacing scanner below. func fixBoldSpacingLine(line string) string { if atxHeadingRe.MatchString(line) { return line } spans := scanInlineCodeSpans(line) if len(spans) == 0 { - line = boldTrailingSpaceRe.ReplaceAllString(line, "$1$2") - line = italicTrailingSpaceRe.ReplaceAllString(line, "$1$2") - return line + return fixEmphasisSpacingSegment(line) } var sb strings.Builder pos := 0 for _, loc := range spans { // Process the non-code segment before this inline code span. seg := line[pos:loc[0]] - seg = boldTrailingSpaceRe.ReplaceAllString(seg, "$1$2") - seg = italicTrailingSpaceRe.ReplaceAllString(seg, "$1$2") - sb.WriteString(seg) + sb.WriteString(fixEmphasisSpacingSegment(seg)) // Preserve inline code span as-is. sb.WriteString(line[loc[0]:loc[1]]) pos = loc[1] } // Remaining non-code segment after the last code span. - seg := line[pos:] - seg = boldTrailingSpaceRe.ReplaceAllString(seg, "$1$2") - seg = italicTrailingSpaceRe.ReplaceAllString(seg, "$1$2") - sb.WriteString(seg) + sb.WriteString(fixEmphasisSpacingSegment(line[pos:])) return sb.String() } +// fixEmphasisSpacingSegment trims only the whitespace immediately inside simple +// *...* and **...** spans. It deliberately ignores runs of 3+ asterisks and +// any candidate whose payload contains another asterisk so nested emphasis-like +// text remains untouched. When both inner sides contain whitespace, single-rune +// payloads are preserved as literal text (for example "* x *" and "** x **"). +func fixEmphasisSpacingSegment(seg string) string { + if !strings.Contains(seg, "*") { + return seg + } + + var sb strings.Builder + pos := 0 + for pos < len(seg) { + openStart, openEnd, ok := nextAsteriskRun(seg, pos) + if !ok { + sb.WriteString(seg[pos:]) + break + } + + sb.WriteString(seg[pos:openStart]) + + markerLen := openEnd - openStart + if markerLen != 1 && markerLen != 2 { + sb.WriteString(seg[openStart:openEnd]) + pos = openEnd + continue + } + + closeStart, closeEnd, ok := nextAsteriskRun(seg, openEnd) + if !ok || closeEnd-closeStart != markerLen { + sb.WriteString(seg[openStart:openEnd]) + pos = openEnd + continue + } + + payload := seg[openEnd:closeStart] + normalized, shouldNormalize := normalizeEmphasisPayload(payload) + if !shouldNormalize { + sb.WriteString(seg[openStart:closeEnd]) + pos = closeEnd + continue + } + + marker := seg[openStart:openEnd] + sb.WriteString(marker) + sb.WriteString(normalized) + sb.WriteString(marker) + pos = closeEnd + } + return sb.String() +} + +func nextAsteriskRun(s string, start int) (runStart, runEnd int, ok bool) { + for i := start; i < len(s); i++ { + if s[i] != '*' { + continue + } + j := i + for j < len(s) && s[j] == '*' { + j++ + } + return i, j, true + } + return 0, 0, false +} + +func normalizeEmphasisPayload(payload string) (string, bool) { + trimmedLeft := strings.TrimLeftFunc(payload, unicode.IsSpace) + trimmed := strings.TrimRightFunc(trimmedLeft, unicode.IsSpace) + if trimmed == "" { + return payload, false + } + + hasLeadingSpace := len(trimmedLeft) != len(payload) + hasTrailingSpace := len(trimmed) != len(trimmedLeft) + if !hasLeadingSpace && !hasTrailingSpace { + return payload, true + } + + if hasLeadingSpace && hasTrailingSpace && utf8.RuneCountInString(trimmed) == 1 { + return payload, false + } + return trimmed, true +} + var setextRe = regexp.MustCompile(`(?m)^([^\n]+)\n(-{3,}\s*$)`) func fixSetextAmbiguity(md string) string { @@ -291,6 +377,44 @@ var contentContainers = [][2]string{ // indented (nested) items. var listItemRe = regexp.MustCompile(`^[ \t]*([-*+]|\d+[.)]) `) +// nestedListIndentRe matches nested list item markers indented with pairs of +// spaces. We rewrite those space pairs to tabs because some downstream +// round-trip paths treat multi-space indented ordered items as flat items or +// literal text, while tab indentation remains nested and avoids 4-space code +// block ambiguity. +var nestedListIndentRe = regexp.MustCompile(`^( {2,})([-*+]|\d+[.)]) `) + +func normalizeNestedListIndentation(md string) string { + lines := strings.Split(md, "\n") + for i, line := range lines { + matches := nestedListIndentRe.FindStringSubmatch(line) + if len(matches) != 3 { + continue + } + if !hasPreviousNonBlankListItem(lines, i) { + continue + } + indent := matches[1] + if len(indent)%2 != 0 { + continue + } + tabs := strings.Repeat("\t", len(indent)/2) + lines[i] = tabs + line[len(indent):] + } + return strings.Join(lines, "\n") +} + +func hasPreviousNonBlankListItem(lines []string, index int) bool { + for i := index - 1; i >= 0; i-- { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" { + return false + } + return listItemRe.MatchString(lines[i]) + } + return false +} + // isListItemOrContinuation returns true for lines that are part of a list: // either a list item marker line or an indented continuation of a list item. // This is used to prevent blank lines being inserted between tight list lines, diff --git a/shortcuts/doc/markdown_fix_test.go b/shortcuts/doc/markdown_fix_test.go index b47ab7785..81ac26a9a 100644 --- a/shortcuts/doc/markdown_fix_test.go +++ b/shortcuts/doc/markdown_fix_test.go @@ -14,6 +14,56 @@ func TestFixBoldSpacing(t *testing.T) { input string want string }{ + { + name: "leading space after opening bold", + input: "** hello**", + want: "**hello**", + }, + { + name: "leading space after opening italic", + input: "* hello*", + want: "*hello*", + }, + { + name: "leading and trailing spaces inside bold are collapsed", + input: "** hello **", + want: "**hello**", + }, + { + name: "leading and trailing spaces inside italic are collapsed", + input: "* hello *", + want: "*hello*", + }, + { + name: "multiple spaced italic spans on one line are each collapsed", + input: "* a* * b*", + want: "*a* *b*", + }, + { + name: "ambiguous italic span stays literal", + input: "2 * x * y", + want: "2 * x * y", + }, + { + name: "ambiguous bold span stays literal", + input: "2 ** x ** y", + want: "2 ** x ** y", + }, + { + name: "single-rune italic with spaces on both sides stays literal", + input: "* x *", + want: "* x *", + }, + { + name: "single-rune bold with spaces on both sides stays literal", + input: "** x **", + want: "** x **", + }, + { + name: "triple-asterisk near miss stays literal", + input: "*** hello**", + want: "*** hello**", + }, { name: "trailing space before closing bold", input: "**hello **", @@ -54,6 +104,16 @@ func TestFixBoldSpacing(t *testing.T) { input: "**foo ** and `**bar **`", want: "**foo** and `**bar **`", }, + { + name: "inline code with spaced italic stays literal while outside span is fixed", + input: "`* hello *` and * hello *", + want: "`* hello *` and *hello*", + }, + { + name: "opening space inside text tag fixed", + input: `** Helpful - 有用性：**`, + want: `**Helpful - 有用性：**`, + }, { name: "double-backtick inline code not modified", input: "``**hello **`` and **world **", @@ -222,6 +282,53 @@ func TestFixTopLevelSoftbreaks(t *testing.T) { } } +func TestNormalizeNestedListIndentation(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "nested ordered list uses tabs instead of space pairs", + input: "1. parent\n 1. child\n 1. grandchild", + want: "1. parent\n\t1. child\n\t\t1. grandchild", + }, + { + name: "nested mixed list markers use tabs instead of space pairs", + input: "- parent\n - child\n 1. grandchild", + want: "- parent\n\t- child\n\t\t1. grandchild", + }, + { + name: "top-level list unchanged", + input: "1. parent\n2. sibling", + want: "1. parent\n2. sibling", + }, + { + name: "indented top-level marker without parent list stays unchanged", + input: "paragraph\n\n 1. item", + want: "paragraph\n\n 1. item", + }, + { + name: "blank-line-separated loose-list sibling stays unchanged", + input: "1. a\n\n 1. b", + want: "1. a\n\n 1. b", + }, + { + name: "indented code block inside list item stays unchanged", + input: "- parent\n\n 1. code", + want: "- parent\n\n 1. code", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := normalizeNestedListIndentation(tt.input) + if got != tt.want { + t.Errorf("normalizeNestedListIndentation(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + func TestFixExportedMarkdown(t *testing.T) { // End-to-end: all fixes applied together input := "# **Title**\nparagraph one\nparagraph two\n**bold **\n> q1\n> q2\nsome text\n---" From 5ecd444ce5ff03109b0a962d73f46dceb31ffff9 Mon Sep 17 00:00:00 2001 From: fangshuyu-768 Date: Mon, 20 Apr 2026 22:03:33 +0800 Subject: [PATCH 7/7] test(doc): harden markdown_fix pipeline with invariant tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 5 invariant-level tests on top of #469's transforms: - TestFixExportedMarkdownIdempotent — f(f(x)) == f(x) across rich fixtures (kitchen sink, CJK, nested containers). Protects the core round-trip promise from future transform interactions that rewrite their own output. - TestFixExportedMarkdownPreservesFencedCodeByteForByte — packs every pipeline-touching shape into a fence and asserts byte-identical output. Code samples must never be silently rewritten by a formatting pass. - TestFixExportedMarkdownPreservesCRLF — CRLF input preserves line endings AND still triggers transforms. Windows-authored markdown should not be silently LF-normalized. - TestFixExportedMarkdownTransformInteractions — composition regressions: nested-list + trailing-space bold, text→list transition, callout containing list with emphasis, heading vs paragraph bold. - TestNormalizeNestedListIndentationDocumentedSkips — locks in the deliberate no-op branches (odd-space indent, blank-line loose-list sibling, 4-space indented code block, parentless two-space) as an explicit spec so future heuristic tweaks surface in the test diff. All transforms, fixtures, and expectations are derived from the head of PR #469. No production code changes. --- shortcuts/doc/markdown_fix_hardening_test.go | 287 +++++++++++++++++++ 1 file changed, 287 insertions(+) create mode 100644 shortcuts/doc/markdown_fix_hardening_test.go diff --git a/shortcuts/doc/markdown_fix_hardening_test.go b/shortcuts/doc/markdown_fix_hardening_test.go new file mode 100644 index 000000000..36264f876 --- /dev/null +++ b/shortcuts/doc/markdown_fix_hardening_test.go @@ -0,0 +1,287 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package doc + +import ( + "strings" + "testing" +) + +// TestFixExportedMarkdownIdempotent asserts the core promise of the exported +// markdown pipeline: applying the fixes twice produces the same result as +// applying them once. Round-trip formatting relies on this invariant, so any +// transform that keeps rewriting its own output would break fetch → edit → +// update → fetch stability. +func TestFixExportedMarkdownIdempotent(t *testing.T) { + fixtures := map[string]string{ + "kitchen sink": strings.Join([]string{ + "# **Title**", + "paragraph one", + "paragraph two", + "**bold ** and * italic*", + "", + "> q1", + "> q2", + "", + "1. parent", + " 1. child", + " 1. grandchild", + "", + "", + "callout body line 1", + "callout body line 2", + "", + "", + "some text", + "---", + "", + "```go", + "// code content with markdown-like shapes must survive as-is", + "**foo **", + "* hello*", + " 1. nested", + "> q", + "---", + "```", + "", + }, "\n"), + + "cjk content": strings.Join([]string{ + "# **测试标题**", + "段落一", + "段落二", + "**有用性 ** and * 关键 *", + "", + "1. 父项", + " 1. 子项", + "", + }, "\n"), + + "nested containers": strings.Join([]string{ + "", + "line a", + "line b", + "", + "", + "", + "quoted 1", + "quoted 2", + "", + "", + }, "\n"), + } + + for name, fixture := range fixtures { + t.Run(name, func(t *testing.T) { + once := fixExportedMarkdown(fixture) + twice := fixExportedMarkdown(once) + if once != twice { + t.Errorf("fixExportedMarkdown is not idempotent for %q\nfirst pass:\n%s\nsecond pass:\n%s", + name, once, twice) + } + }) + } +} + +// TestFixExportedMarkdownPreservesFencedCodeByteForByte packs a fenced code +// block with content that every individual transform in the pipeline would +// normally rewrite, and asserts the fence content comes out byte-for-byte +// identical. This is the pipeline's strongest invariant — users' code samples +// must never be silently modified by a formatting pass. +func TestFixExportedMarkdownPreservesFencedCodeByteForByte(t *testing.T) { + // Every line below is something at least one transform would touch if it + // appeared outside a fence. None of it must change. + dangerous := strings.Join([]string{ + "**foo **", // fixBoldSpacing — trailing space bold + "* hello*", // fixBoldSpacing — leading space italic + "# **heading**", // fixBoldSpacing — redundant heading bold + "para1", // fixTopLevelSoftbreaks — adjacent paragraphs + "para2", + "> q1", // fixBlockquoteHardBreaks — blockquote pair + "> q2", + "some text", // fixSetextAmbiguity — text before --- + "---", + " 1. nested", // normalizeNestedListIndentation + ``, // fixCalloutEmoji — emoji alias + }, "\n") + + // Wrap the dangerous content in a triple-backtick fence and surround with + // content so the pipeline has adjacent regions to potentially touch. + input := "before\n\n```\n" + dangerous + "\n```\n\nafter\n" + + got := fixExportedMarkdown(input) + + // Extract the fence content from the output and compare to the input fence + // content byte-for-byte. + gotFence, ok := extractFirstFenceContent(got) + if !ok { + t.Fatalf("fixExportedMarkdown output lost its fenced code block:\n%s", got) + } + if gotFence != dangerous { + t.Errorf("fenced code content was modified\nwant (bytes): %q\ngot (bytes): %q", + dangerous, gotFence) + } +} + +// extractFirstFenceContent returns the inner text of the first triple-backtick +// fenced code block it finds, or ("", false) if none is present. +func extractFirstFenceContent(md string) (string, bool) { + const fence = "```" + open := strings.Index(md, fence) + if open < 0 { + return "", false + } + // Skip the fence marker and its info-string line. + rest := md[open+len(fence):] + lineEnd := strings.Index(rest, "\n") + if lineEnd < 0 { + return "", false + } + rest = rest[lineEnd+1:] + close := strings.Index(rest, "\n"+fence) + if close < 0 { + return "", false + } + return rest[:close], true +} + +// TestFixExportedMarkdownPreservesCRLF feeds CRLF-terminated markdown (Windows +// line endings) through the pipeline and asserts that line endings are +// preserved AND the emphasis/heading transforms still apply — neither +// silently-LF-normalized nor passed through unchanged. +func TestFixExportedMarkdownPreservesCRLF(t *testing.T) { + lf := "# **Title**\nparagraph one\nparagraph two\n**bold **\n" + crlf := strings.ReplaceAll(lf, "\n", "\r\n") + + got := fixExportedMarkdown(crlf) + + // Transforms must still fire: heading bold stripped, trailing-space bold trimmed. + if strings.Contains(got, "**Title**") { + t.Errorf("heading bold not stripped on CRLF input:\n%q", got) + } + if strings.Contains(got, "**bold **") { + t.Errorf("trailing-space bold not fixed on CRLF input:\n%q", got) + } + // CRLF line endings must survive — we don't want to silently normalize a + // Windows author's document to LF. + if !strings.Contains(got, "\r\n") { + t.Errorf("CRLF line endings were normalized away:\n%q", got) + } +} + +// TestFixExportedMarkdownTransformInteractions covers shapes where more than +// one transform fires on the same input. Each transform is individually tested +// elsewhere; these cases guard against composition regressions. +func TestFixExportedMarkdownTransformInteractions(t *testing.T) { + tests := []struct { + name string + input string + wantContains []string // substrings that must be present after fixes + wantAbsent []string // substrings that must be absent after fixes + }{ + { + name: "nested list item with trailing-space bold", + input: "1. parent\n 1. **child **\n", + wantContains: []string{ + "\t1.", // nested indent converted to tab + "**child**", // trailing space trimmed + }, + wantAbsent: []string{ + " 1.", // original two-space indent gone + "**child **", // original trailing space gone + }, + }, + { + name: "paragraph followed by list", + input: "paragraph\n- item a\n- item b\n", + wantContains: []string{ + "paragraph\n\n- item a", // blank line inserted at text-to-list transition + }, + wantAbsent: []string{ + "\n\n\n", // no triple newline + }, + }, + { + name: "callout containing list with emphasis", + input: "\n- **item **\n- another\n\n", + wantContains: []string{ + "**item**", // trailing-space bold fixed inside callout + }, + wantAbsent: []string{ + "**item **", + }, + }, + { + name: "heading followed by paragraph with bold", + input: "# **Title**\nbody **text **\n", + wantContains: []string{ + "# Title", // heading bold stripped + "body **text**", // paragraph bold trimmed, not stripped + }, + wantAbsent: []string{ + "# **Title**", + "body **text **", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := fixExportedMarkdown(tt.input) + for _, want := range tt.wantContains { + if !strings.Contains(got, want) { + t.Errorf("want substring %q not found in output:\n%s", want, got) + } + } + for _, unwanted := range tt.wantAbsent { + if strings.Contains(got, unwanted) { + t.Errorf("unwanted substring %q still present in output:\n%s", unwanted, got) + } + } + }) + } +} + +// TestNormalizeNestedListIndentationDocumentedSkips locks in the deliberate +// "do nothing" branches of normalizeNestedListIndentation. Each case below is +// a shape the function intentionally does not rewrite; if a future change to +// the heuristic flips one of these, we want the regression to be visible in +// the test diff rather than silently changing user documents. +func TestNormalizeNestedListIndentationDocumentedSkips(t *testing.T) { + tests := []struct { + name string + input string + // want is identical to input — we are asserting "no change". + }{ + { + name: "three-space indent (odd) under list item stays unchanged", + input: "1. parent\n 1. child", + }, + { + name: "five-space indent (odd) under list item stays unchanged", + input: "- parent\n - deep", + }, + { + name: "two-space indent without a parent list item stays unchanged", + input: "plain paragraph\n - not nested", + }, + { + name: "blank-line-separated loose-list sibling stays unchanged", + input: "1. a\n\n 1. b", + }, + { + name: "four-space indented code block under list item stays unchanged", + input: "- parent\n\n 1. code sample", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := normalizeNestedListIndentation(tt.input) + if got != tt.input { + t.Errorf("normalizeNestedListIndentation unexpectedly rewrote documented-skip input\ninput: %q\ngot: %q", tt.input, got) + } + }) + } +}