From da2522c8caadce46e0995b58618bcdfaff281865 Mon Sep 17 00:00:00 2001 From: xuan Date: Tue, 28 Apr 2026 21:17:23 +0800 Subject: [PATCH] feat(pat)!: A-core loop for host-owned PAT (#142) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(pat): A-core minimum viable loop for host-owned PAT Enables third-party agents to integrate PAT via the smallest possible contract surface. The flow a host needs to ship end-to-end: 1. Export DINGTALK_DWS_AGENTCODE in the spawned shell. 2. Run any `dws ...` business command. 3. On PAT hit, CLI exits with code 4 and writes a single-line stderr JSON following docs/pat/contract.md §2. 4. Host parses the JSON and invokes `dws pat chmod ... --agentCode ... --grant-type ...` to grant. 5. Host replays the original command. This PR intentionally ships ONLY the chmod path plus the host-owned switch; the async flow (apply / status / scopes + authRequestId registry + PAT_SCOPE_AUTH_REQUIRED active-request branch) lands in a follow-up stacked PR (A-ext). Scope of A-core: - Host-owned PAT trigger: auth.HostOwnsPATFlow() keyed exclusively on DINGTALK_DWS_AGENTCODE (DINGTALK_AGENT / DWS_CHANNEL / claw-type do NOT participate). - claw-type: hard-wired to "openClaw" in pkg/edition/default.go MergeHeaders hook, matching historical main behavior and decoupled from DINGTALK_AGENT. - dws pat chmod: factory-built (newChmodCommand) so the PAT subcommand tree has no shared package-level caller. - stderr JSON classifier: internal/errors/pat.go covers PAT_NO_PERMISSION / PAT_LOW_RISK / PAT_MEDIUM_RISK / PAT_HIGH_RISK / PAT_SCOPE_* selectors and fills data.hostControl for host consumption. - Env contract: DINGTALK_SESSION_ID / DINGTALK_TRACE_ID / DINGTALK_MESSAGE_ID carry HTTP trace headers (Chain B); DWS_SESSION_ID (+ REWIND_SESSION_ID alias) is the lone fallback for `dws pat chmod --session-id` (Chain A). The two chains are independent; aliases do not cross-pollinate. - Docs: docs/pat/{README,contract,host-integration}.md + refreshed docs/architecture.md "PAT Architecture" chapter + docs/reference.md PAT section. error-catalog.md folded into contract.md §6. - Tests: host-owned signal, stderr contract, classifier, chmod factory, retry / poll loop for the CLI-owned chmod path. Compatibility: no breaking changes on main. All non-PAT commands are untouched. Follow-up PRs (in order): - codex/pat-ext : dws pat apply / status / scopes + AsyncRegistry. - codex/pat-oss-refactor (aka PR B) : oauth_helpers / secure_store refactors split out per code review. Made-with: Cursor * fix(pat): unify host-owned stderr contract * revert: drop PAT docs and changelog from pat core PR * fix(pat): pin behavior auth endpoint * fix(pat): recognize legacy tool miss * fix(pat): fallback on gateway diagnostics * fix(pat): treat authorization uri as opaque * fix(pat): accept result envelope in device-flow polling * feat(pat): add browser policy and poll compatibility * fix(pat): keep CLI and host PAT contracts separate * test(pat): isolate opaque uri retry env * fix(pat): harden PAT result routing * fix(pat): harden host-owned flow and runtime fallback * fix(pat): preserve chmod failure on empty grant result * refactor(pat): drop dead doc anchors, dedupe stderr injection helpers Address two reviewer findings on top of the PAT-core series. Fix: - chmod: resolveSessionIDFromEnv silently selects DWS_SESSION_ID; drop the slog.Warn that emitted both raw session ids into stderr / ~/.dws/logs. - Remove every docs/pat/contract.md / error-catalog.md / host-integration.md anchor from comments, help, and tests; those files were never added by this branch. Comments are now self-contained or point to docs/reference.md where applicable. Refactor: - internal/errors/pat.go: extract lookupCodeIn so getPATErrorCode and getDWSGatewayErrorCode share one traversal; have ClassifyPatAuthCheck delegate to getPATErrorCode instead of repeating the enum walk. - internal/errors/pat.go + internal/app/pat_auth_retry.go: extract ApplyHostMutations as the single injection point for data.hostControl + data.openBrowser; cleanPATJSON and enrichPATErrorForHostControl now share it so the two stderr-JSON write paths cannot drift. - internal/app/pat_auth_retry.go: drop buildPATScopeHostJSON and buildHostControlState dead 1-line wrappers. - internal/pat/chmod.go: drop unused patApply/Status/Scopes constants; derive legacyToolArgs from toolArgs by clone + scopes->scope rename so the two payloads stay in lock-step on every other field. Net 15 files, +176 / -193. Verification: env -u DINGTALK_DWS_AGENTCODE go test \ ./internal/errors ./internal/auth ./internal/pat ./test/unit env -u DINGTALK_DWS_AGENTCODE go test ./internal/app \ -run 'Test(IsPat|ExtractPat|PrintPat|PollPat|HandlePat|RetryWithPat|EnrichPAT|BuildPAT|DirectRuntime|ResolveIdentityHeaders)' env -u DINGTALK_DWS_AGENTCODE go test -race \ ./internal/pat ./internal/errors ./test/unit env -u DINGTALK_DWS_AGENTCODE go test -race ./internal/app \ -run 'Test(HandlePat|RetryWithPat|EnrichPAT|BuildPAT|ResolveIdentityHeaders|DirectRuntime)' go vet ./... && go build ./... All green. Made-with: Cursor --------- Co-authored-by: shangguanxuan.sgx --- docs/reference.md | 3 +- internal/app/direct_runtime.go | 107 ++++- internal/app/direct_runtime_test.go | 70 ++- internal/app/pat_auth_retry.go | 190 ++++++-- internal/app/pat_auth_retry_test.go | 504 ++++++++++++++++++- internal/app/pat_hostcontrol_wire.go | 62 +++ internal/app/runner.go | 46 +- internal/app/runner_test.go | 11 + internal/auth/channel.go | 37 ++ internal/errors/errors.go | 6 +- internal/errors/errors_extra_test.go | 2 +- internal/errors/errors_test.go | 2 +- internal/errors/pat.go | 196 ++++++-- internal/errors/pat_test.go | 274 ++++++++++- internal/pat/browser_policy.go | 228 +++++++++ internal/pat/browser_policy_test.go | 237 +++++++++ internal/pat/chmod.go | 330 ++++++++++++- internal/pat/chmod_test.go | 613 ++++++++++++++++++++++++ internal/pat/pat.go | 29 +- pkg/edition/default.go | 17 +- test/unit/open_source_policy_test.go | 22 +- test/unit/pat_host_owned_signal_test.go | 102 ++++ test/unit/pat_stderr_contract_test.go | 268 +++++++++++ 23 files changed, 3230 insertions(+), 126 deletions(-) create mode 100644 internal/app/pat_hostcontrol_wire.go create mode 100644 internal/auth/channel.go create mode 100644 internal/pat/browser_policy.go create mode 100644 internal/pat/browser_policy_test.go create mode 100644 internal/pat/chmod_test.go create mode 100644 test/unit/pat_host_owned_signal_test.go create mode 100644 test/unit/pat_stderr_contract_test.go diff --git a/docs/reference.md b/docs/reference.md index 2c37b8e0..5e40bc2c 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -19,8 +19,9 @@ | 1 | API | MCP tool call or upstream API failure / MCP 工具调用或上游 API 失败 | | 2 | Auth | Authentication or authorization failure / 身份认证或授权失败 | | 3 | Validation | Invalid input, flags, or parameter schema mismatch / 输入参数校验失败 | -| 4 | Discovery | Server discovery, cache, or protocol negotiation failure / 服务发现失败 | +| 4 | PAT | PAT authorization interception; stderr carries raw machine-readable PAT JSON / PAT 授权拦截；stderr 返回原始机器可解析 JSON | | 5 | Internal | Unexpected internal error / 未预期的内部错误 | +| 6 | Discovery | Server discovery, cache, or protocol negotiation failure / 服务发现、缓存或协议协商失败 | With `-f json`, error responses include structured payloads: `category`, `reason`, `hint`, `actions`. diff --git a/internal/app/direct_runtime.go b/internal/app/direct_runtime.go index b95a772b..c7e159e3 100644 --- a/internal/app/direct_runtime.go +++ b/internal/app/direct_runtime.go @@ -14,10 +14,13 @@ package app import ( + "net" + "net/url" "os" "strings" "sync" + authpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/auth" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cli" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" @@ -38,6 +41,57 @@ var legacyDirectRuntimeAliases = map[string]string{ "dingtalk-ai-sincere-hire": "ai-sincere-hire", } +const ( + defaultPATProductID = "pat" + defaultPATDisplayName = "行为授权" + defaultPATServerID = "abc3c880fb90f04b52d1426aaf093766e5fc9ec38411688cbb74df42a584d374" +) + +func defaultPATServerDescriptor() market.ServerDescriptor { + return market.ServerDescriptor{ + Key: defaultPATProductID, + DisplayName: defaultPATDisplayName, + Endpoint: defaultPATMCPEndpoint(), + CLI: market.CLIOverlay{ + ID: defaultPATProductID, + Command: defaultPATProductID, + Prefixes: []string{defaultPATProductID}, + }, + } +} + +func defaultPATMCPEndpoint() string { + return defaultPATGatewayBaseURL() + "/server/" + defaultPATServerID +} + +func defaultPATGatewayBaseURL() string { + raw := strings.TrimSpace(authpkg.GetMCPBaseURL()) + parsed, err := url.Parse(raw) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return strings.TrimRight(raw, "/") + } + + host := parsed.Hostname() + switch { + case host == "mcp.dingtalk.com": + host = "mcp-gw.dingtalk.com" + case strings.HasPrefix(host, "pre-mcp."): + host = strings.Replace(host, "pre-mcp.", "pre-mcp-gw.", 1) + case strings.HasPrefix(host, "mcp."): + host = strings.Replace(host, "mcp.", "mcp-gw.", 1) + } + + if port := parsed.Port(); port != "" { + parsed.Host = net.JoinHostPort(host, port) + } else { + parsed.Host = host + } + parsed.Path = strings.TrimRight(parsed.Path, "/") + parsed.RawQuery = "" + parsed.Fragment = "" + return strings.TrimRight(parsed.String(), "/") +} + // SetDynamicServers injects server data discovered from servers.json. // All product endpoints are resolved dynamically from this data. func SetDynamicServers(servers []market.ServerDescriptor) { @@ -48,6 +102,7 @@ func SetDynamicServers(servers []market.ServerDescriptor) { products := make(map[string]bool) aliases := make(map[string]string) toolEndpoints := make(map[string]string) + registerDynamicServer(defaultPATServerDescriptor(), endpoints, products, aliases, toolEndpoints) for _, server := range servers { if server.CLI.Skip { continue @@ -102,6 +157,47 @@ func SetDynamicServers(servers []market.ServerDescriptor) { dynamicToolEndpoints = toolEndpoints } +func registerDynamicServer(server market.ServerDescriptor, endpoints map[string]string, products map[string]bool, aliases map[string]string, toolEndpoints map[string]string) { + if server.CLI.Skip { + return + } + id := strings.TrimSpace(server.CLI.ID) + endpoint := strings.TrimSpace(server.Endpoint) + if id != "" && endpoint != "" { + endpoints[id] = endpoint + products[id] = true + } + cmd := strings.TrimSpace(server.CLI.Command) + if cmd != "" && cmd != id && endpoint != "" { + endpoints[cmd] = endpoint + products[cmd] = true + } + for _, alias := range server.CLI.Aliases { + alias = strings.TrimSpace(alias) + if alias != "" && endpoint != "" { + endpoints[alias] = endpoint + products[alias] = true + // Build alias -> CLI.ID mapping. + aliases[alias] = id + } + } + // Build tool -> endpoint mapping from CLI tools and overrides. + if endpoint != "" { + for _, tool := range server.CLI.Tools { + toolName := strings.TrimSpace(tool.Name) + if toolName != "" { + toolEndpoints[toolName] = endpoint + } + } + for toolName := range server.CLI.ToolOverrides { + toolName = strings.TrimSpace(toolName) + if toolName != "" { + toolEndpoints[toolName] = endpoint + } + } + } +} + func shouldUseDirectRuntime(invocation executor.Invocation) bool { if strings.TrimSpace(os.Getenv(cli.CatalogFixtureEnv)) != "" { return false @@ -149,6 +245,14 @@ func directRuntimeEndpoint(productID, toolName string) (string, bool) { } } } + + // Priority 3: built-in PAT fallback for cold-start paths that run before + // discovery/plugin registration has populated the dynamic registry. + for _, candidate := range []string{strings.TrimSpace(productID), normalized} { + if candidate == defaultPATProductID { + return defaultPATMCPEndpoint(), true + } + } return "", false } @@ -158,7 +262,8 @@ func DirectRuntimeProductIDs() map[string]bool { dynamicMu.RLock() dp := dynamicProducts dynamicMu.RUnlock() - ids := make(map[string]bool, len(dp)) + ids := make(map[string]bool, len(dp)+1) + ids[defaultPATProductID] = true for key := range dp { ids[key] = true } diff --git a/internal/app/direct_runtime_test.go b/internal/app/direct_runtime_test.go index 8fc04f09..9fd90c4b 100644 --- a/internal/app/direct_runtime_test.go +++ b/internal/app/direct_runtime_test.go @@ -1,6 +1,74 @@ package app -import "testing" +import ( + "os" + "path/filepath" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" +) + +func TestDefaultPATServerDescriptorUsesBehaviorAuthorizationName(t *testing.T) { + server := defaultPATServerDescriptor() + if server.CLI.ID != "pat" { + t.Fatalf("default PAT server id = %q, want pat", server.CLI.ID) + } + if server.DisplayName != "行为授权" { + t.Fatalf("default PAT server display name = %q, want 行为授权", server.DisplayName) + } + if server.Endpoint != defaultPATMCPEndpoint() { + t.Fatalf("default PAT server endpoint = %q, want %q", server.Endpoint, defaultPATMCPEndpoint()) + } +} + +func TestDirectRuntimeProductIDsIncludesDefaultPAT(t *testing.T) { + dynamicMu.Lock() + previousProducts := dynamicProducts + dynamicProducts = nil + dynamicMu.Unlock() + t.Cleanup(func() { + dynamicMu.Lock() + dynamicProducts = previousProducts + dynamicMu.Unlock() + }) + + ids := DirectRuntimeProductIDs() + if !ids["pat"] { + t.Fatalf("DirectRuntimeProductIDs() missing default pat product: %#v", ids) + } +} + +func TestDirectRuntimeEndpoint_DefaultPATFallbackWhenRegistryMissing(t *testing.T) { + withCleanDynamicRegistry(t) + assertEndpoint(t, "pat", "", defaultPATMCPEndpoint()) +} + +func TestDirectRuntimeEndpoint_DefaultPATFallbackUsesConfiguredMCPBaseURL(t *testing.T) { + withCleanDynamicRegistry(t) + + tmpDir := t.TempDir() + if err := os.WriteFile(filepath.Join(tmpDir, "mcp_url"), []byte("http://127.0.0.1:54321/base"), 0o600); err != nil { + t.Fatalf("WriteFile(mcp_url) error = %v", err) + } + t.Setenv("DWS_CONFIG_DIR", tmpDir) + + assertEndpoint(t, "pat", "", "http://127.0.0.1:54321/base/server/"+defaultPATServerID) +} + +func TestDirectRuntimeEndpoint_PATDiscoveryOverrideWinsOverBuiltInFallback(t *testing.T) { + withCleanDynamicRegistry(t) + customEndpoint := "https://example.com/server/custom-pat" + SetDynamicServers([]market.ServerDescriptor{ + { + Endpoint: customEndpoint, + CLI: market.CLIOverlay{ + ID: "pat", + Command: "pat", + }, + }, + }) + assertEndpoint(t, "pat", "", customEndpoint) +} func TestNormalizeDirectRuntimeProductIDPreservesLegacyHiddenVendorRouting(t *testing.T) { dynamicMu.Lock() diff --git a/internal/app/pat_auth_retry.go b/internal/app/pat_auth_retry.go index f31bd053..21725da4 100644 --- a/internal/app/pat_auth_retry.go +++ b/internal/app/pat_auth_retry.go @@ -32,6 +32,7 @@ import ( authpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/auth" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pat" "github.com/fatih/color" ) @@ -43,8 +44,12 @@ const ( // PatAuthPollInterval is how often we poll to check if the user has // completed authorization. PatAuthPollInterval = 5 * time.Second + + patScopeAuthRequiredCode = "PAT_SCOPE_AUTH_REQUIRED" ) +var openBrowserFunc = tryOpenBrowser + // PatScopeError holds information about a missing PAT scope. type PatScopeError struct { OriginalError string @@ -182,21 +187,50 @@ func PrintPatAuthError(w io.Writer, scopeErr *PatScopeError) { // PrintPatAuthJSON prints a machine-readable PAT authorization error. func PrintPatAuthJSON(w io.Writer, scopeErr *PatScopeError) { - payload := map[string]any{ - "ok": false, - "identity": scopeErr.Identity, - "error": map[string]any{ - "type": scopeErr.ErrorType, - "message": scopeErr.Message, - "hint": scopeErr.Hint, - }, + fmt.Fprintln(w, buildPATScopeJSON(scopeErr, authpkg.HostOwnsPATFlow())) +} + +func wantsStructuredPATOutput(r *runtimeRunner) bool { + if r == nil || r.globalFlags == nil { + return false } - if scopeErr.MissingScope != "" { - payload["missing_scope"] = scopeErr.MissingScope + return strings.EqualFold(strings.TrimSpace(r.globalFlags.Format), "json") +} + +func wantsStructuredPATOutputFromRunner(runner executor.Runner) bool { + rr, ok := runner.(*runtimeRunner) + if !ok { + return false + } + return wantsStructuredPATOutput(rr) +} + +func currentPATOpenBrowser(configDir string) bool { + return pat.EffectiveOpenBrowser(configDir) +} + +func enrichPATErrorWithOpenBrowser(raw string, openBrowser bool) string { + if strings.TrimSpace(raw) == "" { + return raw } - data, _ := json.MarshalIndent(payload, "", " ") - fmt.Fprintln(w, string(data)) + var payload map[string]any + if err := json.Unmarshal([]byte(raw), &payload); err != nil { + return raw + } + + data, ok := payload["data"].(map[string]any) + if !ok || data == nil { + data = map[string]any{} + payload["data"] = data + } + data["openBrowser"] = openBrowser + + encoded, err := json.Marshal(payload) + if err != nil { + return raw + } + return string(encoded) } // WaitForPatAuthorization polls until the user completes authorization or timeout. @@ -258,6 +292,19 @@ func WaitForPatAuthorization(ctx context.Context, configDir string, output io.Wr // retryWithPatAuthRetry wraps an invocation that failed with a PAT scope error. // It waits for the user to complete authorization and then retries the invocation. func retryWithPatAuthRetry(ctx context.Context, runner executor.Runner, invocation executor.Invocation, scopeErr *PatScopeError, configDir string, output io.Writer) (executor.Result, error) { + hostOwnedPAT := authpkg.HostOwnsPATFlow() + slog.Debug("pat.host_owned_decision", + "site", "retryWithPatAuthRetry", + "hostOwned", hostOwnedPAT, + "agentCodeEnvSet", os.Getenv(authpkg.AgentCodeEnv) != "", + ) + if hostOwnedPAT { + return executor.Result{}, &apperrors.PATError{RawJSON: buildPATScopeJSON(scopeErr, true)} + } + if wantsStructuredPATOutputFromRunner(runner) { + return executor.Result{}, &apperrors.PATError{RawJSON: buildPATScopeJSON(scopeErr, false)} + } + // Print the PAT error in human-readable format PrintPatAuthError(output, scopeErr) @@ -293,8 +340,6 @@ const ( patPollTimeout = 10 * time.Minute ) -var openBrowserFunc = tryOpenBrowser - // patRetryingKey is a context key to prevent recursive PAT auth checks. // After APPROVED, the retry should not trigger another PAT flow. type patRetryingKeyType struct{} @@ -366,9 +411,17 @@ func handlePatAuthCheck( "flowId", patData.Data.FlowID, "hasSecret", patData.Data.ClientSecret != "", ) + hostOwnedPAT := authpkg.HostOwnsPATFlow() + openBrowser := currentPATOpenBrowser(configDir) + slog.Debug("pat.host_owned_decision", + "site", "handlePatAuthCheck", + "hostOwned", hostOwnedPAT, + "agentCodeEnvSet", os.Getenv(authpkg.AgentCodeEnv) != "", + ) // Inject clientId/clientSecret from PAT response as runtime credentials // so that subsequent device flow auth uses the server-assigned app identity. + var appCfg *authpkg.AppConfig if patData.Data.ClientID != "" { if patData.Data.ClientSecret != "" { // When both clientId and clientSecret are provided, use direct mode @@ -381,19 +434,32 @@ func handlePatAuthCheck( authpkg.SetClientIDFromMCP(patData.Data.ClientID) } - // Persist clientId (and optionally secret) to ~/.dws/app.json so that - // future process invocations can load it at startup and populate - // DWS_CLIENT_ID env before the first MCP request. - appCfg := &authpkg.AppConfig{ - ClientID: patData.Data.ClientID, - } + // Persist only after an explicit APPROVED result below. Raw PAT + // interceptions (host-owned / json / empty-flow pass-through) must not + // rewrite the shared ~/.dws/app.json state for unrelated shells or agents. + appCfg = &authpkg.AppConfig{ClientID: patData.Data.ClientID} if patData.Data.ClientSecret != "" { appCfg.ClientSecret = authpkg.PlainSecret(patData.Data.ClientSecret) } - if err := authpkg.SaveAppConfig(configDir, appCfg); err != nil { - slog.Warn("failed to persist app config from PAT", "error", err) - fmt.Fprintf(output, " \u26a0 保存应用配置失败: %v (下次启动可能需要重新授权)\n", err) + } + + // In host-controlled PAT mode (driven solely by DINGTALK_DWS_AGENTCODE), + // or when flowId is absent, the CLI returns machine-readable JSON to + // stderr and leaves UI/polling/retry to the host. `claw-type` is NOT + // used for this decision — it is only forwarded on the wire via + // edition.MergeHeaders and surfaced in hostControl for traceability. + if hostOwnedPAT || patData.Data.FlowID == "" { + if hostOwnedPAT { + return executor.Result{}, &apperrors.PATError{RawJSON: enrichPATErrorForHostControl(patErr.RawJSON)} } + return executor.Result{}, &apperrors.PATError{RawJSON: enrichPATErrorWithOpenBrowser(patErr.RawJSON, openBrowser)} + } + + if wantsStructuredPATOutput(r) { + if openBrowser && patData.Data.URI != "" { + _ = openBrowserFunc(patData.Data.URI) + } + return executor.Result{}, &apperrors.PATError{RawJSON: enrichPATErrorWithOpenBrowser(patErr.RawJSON, openBrowser)} } bold := color.New(color.Bold).SprintFunc() @@ -410,14 +476,9 @@ func handlePatAuthCheck( } if patData.Data.URI != "" { fmt.Fprintf(output, " %s %s\n\n", dim("🔗"), cyan(patData.Data.URI)) - // Best-effort browser open. - _ = openPATAuthorizationURI(patData.Data.URI) - } - - // If no flowId, we can't poll — fall back to returning PATError for host-app. - if patData.Data.FlowID == "" { - fmt.Fprintln(output) - return executor.Result{}, patErr + if openBrowser { + _ = openPATAuthorizationURI(patData.Data.URI) + } } // Poll the device flow status until user authorizes, rejects, or timeout. @@ -439,6 +500,13 @@ func handlePatAuthCheck( fmt.Fprintf(output, "%s %s\n", greenFn("✓"), bold("授权成功!")) fmt.Fprintln(output) + if appCfg != nil { + if err := authpkg.SaveAppConfig(configDir, appCfg); err != nil { + slog.Warn("failed to persist approved app config from PAT", "error", err) + fmt.Fprintf(output, " \u26a0 保存应用配置失败: %v (下次启动可能需要重新授权)\n", err) + } + } + // Exchange authCode for a fresh access token (mirrors device_flow loginOnce). if authCode != "" { slog.Debug("PAT retry: exchanging authCode for token", "hasCode", true) @@ -503,6 +571,66 @@ func handlePatAuthCheck( } } +func enrichPATErrorForHostControl(raw string) string { + if strings.TrimSpace(raw) == "" { + return raw + } + + var payload map[string]any + if err := json.Unmarshal([]byte(raw), &payload); err != nil { + return raw + } + + // Route back through the classifier so host-owned active retry emits the + // exact same PAT JSON shape as passive classification. + if patErr := apperrors.ClassifyPatAuthCheck(payload); patErr != nil { + return patErr.RawJSON + } + + apperrors.ApplyHostMutations(payload) + + // stderr JSON MUST be single-line. + encoded, err := json.Marshal(payload) + if err != nil { + return raw + } + return string(encoded) +} + +// buildPATScopeJSON renders the PAT_SCOPE_AUTH_REQUIRED stderr payload. +// includeHostControl=true follows the standard host-owned/CLI-owned split +// (data.hostControl is injected only if HostControlBlock is non-nil). +// includeHostControl=false is an explicit override used by the CLI-owned +// branch so that any env-mode misconfiguration cannot leak a host-owned +// contract into stderr. +func buildPATScopeJSON(scopeErr *PatScopeError, includeHostControl bool) string { + data := map[string]any{ + "identity": scopeErr.Identity, + "errorType": scopeErr.ErrorType, + "message": scopeErr.Message, + "hint": scopeErr.Hint, + "missingScope": scopeErr.MissingScope, + "openBrowser": apperrors.PATOpenBrowserValue(), + } + if includeHostControl { + if hostControl := apperrors.HostControlBlock(); hostControl != nil { + data["hostControl"] = hostControl + } + } + + payload := map[string]any{ + "success": false, + "code": patScopeAuthRequiredCode, + "data": data, + } + // stderr JSON MUST be single-line. + b, err := json.Marshal(payload) + if err != nil { + return `{"success":false,"code":"PAT_SCOPE_AUTH_REQUIRED"}` + } + return string(b) +} + // pollPatDeviceFlow polls the PAT device flow status endpoint until a terminal // state (APPROVED/REJECTED/EXPIRED) is reached or the context is cancelled. // Returns the final status string and the authCode (non-empty only on APPROVED). diff --git a/internal/app/pat_auth_retry_test.go b/internal/app/pat_auth_retry_test.go index ba7d8299..1c639526 100644 --- a/internal/app/pat_auth_retry_test.go +++ b/internal/app/pat_auth_retry_test.go @@ -29,6 +29,7 @@ import ( authpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/auth" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pat" ) func TestIsPatScopeError_MissingScope(t *testing.T) { @@ -86,21 +87,6 @@ func TestIsPatScopeError_InsufficientScope(t *testing.T) { } } -func TestExtractPatScopeError_MissingScope(t *testing.T) { - t.Parallel() - err := apperrors.NewAuth("missing required scope(s): mail:user_mailbox.message:send") - scopeErr := extractPatScopeError(err) - if scopeErr == nil { - t.Fatal("expected non-nil PatScopeError") - } - if scopeErr.ErrorType != "missing_scope" { - t.Errorf("expected error type 'missing_scope', got %q", scopeErr.ErrorType) - } - if !strings.Contains(scopeErr.Hint, "dws auth login") { - t.Errorf("expected hint to contain 'dws auth login', got %q", scopeErr.Hint) - } -} - func TestExtractPatScopeError_ExtractsScope(t *testing.T) { t.Parallel() err := &PatScopeError{ @@ -142,7 +128,7 @@ func TestPrintPatAuthError_HumanReadable(t *testing.T) { } func TestPrintPatAuthJSON_MachineReadable(t *testing.T) { - t.Parallel() + t.Setenv(authpkg.AgentCodeEnv, "") var buf strings.Builder scopeErr := &PatScopeError{ Identity: "user", @@ -153,12 +139,28 @@ func TestPrintPatAuthJSON_MachineReadable(t *testing.T) { } PrintPatAuthJSON(&buf, scopeErr) + // The payload is required to be single-line; assert by parsing the JSON + // rather than by matching pretty-printed substrings. output := buf.String() - if !strings.Contains(output, `"ok": false`) { - t.Errorf("expected JSON to contain ok: false, got: %s", output) + var parsed map[string]any + if err := json.Unmarshal([]byte(output), &parsed); err != nil { + t.Fatalf("PrintPatAuthJSON must emit directly-parsable JSON: %v\nraw=%s", err, output) } - if !strings.Contains(output, `"missing_scope": "mail:send"`) { - t.Errorf("expected JSON to contain missing_scope, got: %s", output) + if code, _ := parsed["code"].(string); code != "PAT_SCOPE_AUTH_REQUIRED" { + t.Errorf("code = %q, want PAT_SCOPE_AUTH_REQUIRED", code) + } + data, _ := parsed["data"].(map[string]any) + if data == nil { + t.Fatalf("expected data object, got: %s", output) + } + if got, _ := data["missingScope"].(string); got != "mail:send" { + t.Errorf("missingScope = %q, want mail:send", got) + } + if got, ok := data["openBrowser"].(bool); !ok || !got { + t.Errorf("openBrowser = %#v, want true", data["openBrowser"]) + } + if _, ok := data["hostControl"]; ok { + t.Errorf("unexpected data.hostControl in CLI-owned JSON output: %s", output) } } @@ -466,7 +468,6 @@ func TestPollPatDeviceFlow_UnknownStatusHidesRawResponseByDefault(t *testing.T) t.Fatalf("expected raw poll response to stay hidden by default, got %q", output) } } - func TestPollPatDeviceFlow_ResultEnvelopeCompatibility(t *testing.T) { server, configDir := setupPollServer(t, []authpkg.DevicePollResponse{ {Success: true, Result: authpkg.DevicePollData{Status: "PENDING"}}, @@ -590,6 +591,7 @@ func makePATErrorJSONWithURI(flowID, clientID, uri string) string { } func TestHandlePatAuthCheck_Approved(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") server, configDir := setupHandlePATServer(t, "APPROVED", "test-auth-code") defer server.Close() @@ -622,6 +624,9 @@ func TestHandlePatAuthCheck_Approved(t *testing.T) { if !retryHasKey { t.Fatal("expected retry context to have patRetryingKey") } + if _, err := os.Stat(filepath.Join(configDir, "app.json")); err != nil { + t.Fatalf("expected approved PAT flow to persist app.json, stat error = %v", err) + } // Verify SetClientIDFromMCP was called with the PAT response clientId. if cid := authpkg.ClientID(); cid != "test-client-id" { t.Errorf("expected ClientID 'test-client-id', got %q", cid) @@ -629,6 +634,7 @@ func TestHandlePatAuthCheck_Approved(t *testing.T) { } func TestHandlePatAuthCheck_Rejected(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") server, configDir := setupHandlePATServer(t, "REJECTED", "") defer server.Close() @@ -657,7 +663,132 @@ func TestHandlePatAuthCheck_Rejected(t *testing.T) { } } +func TestHandlePatAuthCheck_HostControlledFlowIDPassthrough(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", tmpDir) + // Host-owned decision: driven ONLY by DINGTALK_DWS_AGENTCODE. + // DINGTALK_AGENT is set to demonstrate it does NOT leak into + // hostControl.clawType — the open-source build pins that to the + // literal edition.DefaultOSSClawType value ("openClaw"). + t.Setenv(authpkg.AgentCodeEnv, "agt-sales") + t.Setenv("DINGTALK_AGENT", "sales-copilot") + + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + t.Fatal("runner should not be called in host-controlled PAT mode") + return executor.Result{}, nil + }, + } + + runner := &runtimeRunner{fallback: mock} + patErr := &apperrors.PATError{RawJSON: makePATErrorJSON("flow-host", "test-client-id")} + + ctx := context.Background() + var buf bytes.Buffer + _, err := handlePatAuthCheck(ctx, runner, executor.Invocation{ + CanonicalProduct: "test", + Tool: "test_tool", + }, patErr, tmpDir, &buf) + + if err == nil { + t.Fatal("expected PATError in host-controlled mode") + } + patOut, ok := err.(*apperrors.PATError) + if !ok { + t.Fatalf("expected *PATError, got %T: %v", err, err) + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output in host mode, got %q", got) + } + if _, err := os.Stat(filepath.Join(tmpDir, "app.json")); !os.IsNotExist(err) { + t.Fatalf("host-owned PAT must not persist shared app.json, stat error = %v", err) + } + + var payload map[string]any + if err := json.Unmarshal([]byte(patOut.RawJSON), &payload); err != nil { + t.Fatalf("json.Unmarshal(host PAT payload) error = %v\nraw=%s", err, patOut.RawJSON) + } + + data, _ := payload["data"].(map[string]any) + if got, _ := data["flowId"].(string); got != "flow-host" { + t.Fatalf("data.flowId = %q, want flow-host", got) + } + hostControl, _ := data["hostControl"].(map[string]any) + if got, _ := hostControl["clawType"].(string); got != "openClaw" { + t.Fatalf("hostControl.clawType = %q, want openClaw (hard-wired by open-source edition)", got) + } + if got, _ := hostControl["callbackOwner"].(string); got != "host" { + t.Fatalf("hostControl.callbackOwner = %q, want host", got) + } + if _, ok := data["callbacks"]; ok { + t.Fatalf("unexpected callbacks contract in host-controlled PAT payload: %#v", data["callbacks"]) + } + if _, ok := payload["_meta"]; ok { + t.Fatalf("unexpected _meta contract in host-controlled PAT payload: %#v", payload["_meta"]) + } + if strings.Contains(patOut.RawJSON, `"pat","callback"`) { + t.Fatalf("host PAT payload should not advertise dws pat callback argv: %s", patOut.RawJSON) + } +} + +func TestHandlePatAuthCheck_HostControlledEmptyFlowID_StillReturnsContract(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", tmpDir) + t.Setenv(authpkg.AgentCodeEnv, "agt-support") + t.Setenv("DINGTALK_AGENT", "customer-support") + + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + t.Fatal("runner should not be called in host-controlled PAT mode") + return executor.Result{}, nil + }, + } + + runner := &runtimeRunner{fallback: mock} + patErr := &apperrors.PATError{RawJSON: makePATErrorJSON("", "test-client-id")} + + var buf bytes.Buffer + _, err := handlePatAuthCheck(context.Background(), runner, executor.Invocation{ + CanonicalProduct: "test", + Tool: "test_tool", + }, patErr, tmpDir, &buf) + + if err == nil { + t.Fatal("expected PATError in host-controlled mode") + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output in host mode, got %q", got) + } + if _, err := os.Stat(filepath.Join(tmpDir, "app.json")); !os.IsNotExist(err) { + t.Fatalf("host-owned PAT must not persist shared app.json, stat error = %v", err) + } + patOut, ok := err.(*apperrors.PATError) + if !ok { + t.Fatalf("expected *PATError, got %T: %v", err, err) + } + + var payload map[string]any + if err := json.Unmarshal([]byte(patOut.RawJSON), &payload); err != nil { + t.Fatalf("json.Unmarshal(host PAT payload) error = %v\nraw=%s", err, patOut.RawJSON) + } + data, _ := payload["data"].(map[string]any) + hostControl, _ := data["hostControl"].(map[string]any) + if got, _ := hostControl["callbackOwner"].(string); got != "host" { + t.Fatalf("hostControl.callbackOwner = %q, want host", got) + } + if _, ok := data["callbacks"]; ok { + t.Fatalf("unexpected callbacks contract when flowId is absent: %#v", data["callbacks"]) + } + if _, ok := payload["_meta"]; ok { + t.Fatalf("unexpected _meta contract when flowId is absent: %#v", payload["_meta"]) + } + if strings.Contains(patOut.RawJSON, `"pat","callback"`) { + t.Fatalf("host PAT payload should not advertise dws pat callback argv: %s", patOut.RawJSON) + } +} + func TestHandlePatAuthCheck_EmptyFlowID_FallsBackToPATError(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") // No poll server needed — empty flowId means no polling, return PATError directly. tmpDir := t.TempDir() t.Setenv("DWS_CONFIG_DIR", tmpDir) @@ -686,9 +817,340 @@ func TestHandlePatAuthCheck_EmptyFlowID_FallsBackToPATError(t *testing.T) { if _, ok := err.(*apperrors.PATError); !ok { t.Errorf("expected *PATError, got %T: %v", err, err) } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output for raw PAT passthrough, got %q", got) + } +} + +func TestHandlePatAuthCheck_JSONModeReturnsStructuredPATErrorWithoutRetry(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") + tmpDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", tmpDir) + if _, err := pat.SetBrowserPolicy(tmpDir, "", false); err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + t.Fatal("runner should not be called in json PAT mode") + return executor.Result{}, nil + }, + } + + runner := &runtimeRunner{ + fallback: mock, + globalFlags: &GlobalFlags{Format: "json"}, + } + patErr := &apperrors.PATError{RawJSON: makePATErrorJSON("flow-json", "test-client-id")} + + var buf bytes.Buffer + _, err := handlePatAuthCheck(context.Background(), runner, executor.Invocation{ + CanonicalProduct: "test", + Tool: "test_tool", + }, patErr, tmpDir, &buf) + + if err == nil { + t.Fatal("expected PATError in json PAT mode") + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output in json PAT mode, got %q", got) + } + if _, err := os.Stat(filepath.Join(tmpDir, "app.json")); !os.IsNotExist(err) { + t.Fatalf("json PAT mode must not persist shared app.json, stat error = %v", err) + } + + patOut, ok := err.(*apperrors.PATError) + if !ok { + t.Fatalf("expected *PATError, got %T: %v", err, err) + } + var payload map[string]any + if err := json.Unmarshal([]byte(patOut.RawJSON), &payload); err != nil { + t.Fatalf("json.Unmarshal(json PAT payload) error = %v\nraw=%s", err, patOut.RawJSON) + } + data, _ := payload["data"].(map[string]any) + if got, ok := data["openBrowser"].(bool); !ok || got { + t.Fatalf("data.openBrowser = %#v, want false", data["openBrowser"]) + } + if _, ok := data["hostControl"]; ok { + t.Fatalf("unexpected data.hostControl in CLI-owned json PAT mode: %s", patOut.RawJSON) + } +} + +func TestHandlePatAuthCheck_JSONModeCanOpenBrowserWithoutTextOutput(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") + tmpDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", tmpDir) + if _, err := pat.SetBrowserPolicy(tmpDir, "", true); err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + + var opened string + origOpenBrowser := openBrowserFunc + openBrowserFunc = func(rawURL string) error { + opened = rawURL + return nil + } + t.Cleanup(func() { openBrowserFunc = origOpenBrowser }) + + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + t.Fatal("runner should not be called in json PAT mode") + return executor.Result{}, nil + }, + } + + runner := &runtimeRunner{ + fallback: mock, + globalFlags: &GlobalFlags{Format: "json"}, + } + raw := `{"code":"AGENT_CODE_NOT_EXISTS","data":{"desc":"test auth","flowId":"flow-json","uri":"https://example.com/pat","clientId":"test-client-id"}}` + + var buf bytes.Buffer + _, err := handlePatAuthCheck(context.Background(), runner, executor.Invocation{ + CanonicalProduct: "test", + Tool: "test_tool", + }, &apperrors.PATError{RawJSON: raw}, tmpDir, &buf) + + if err == nil { + t.Fatal("expected PATError in json PAT mode") + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output in json PAT mode, got %q", got) + } + if _, err := os.Stat(filepath.Join(tmpDir, "app.json")); !os.IsNotExist(err) { + t.Fatalf("json PAT mode must not persist shared app.json, stat error = %v", err) + } + if opened != "https://example.com/pat" { + t.Fatalf("opened url = %q, want https://example.com/pat", opened) + } +} + +func TestHandlePatAuthCheck_NonJSONModeRespectsBrowserPolicy(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") + server, configDir := setupHandlePATServer(t, "APPROVED", "test-auth-code") + defer server.Close() + if _, err := pat.SetBrowserPolicy(configDir, "", false); err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + + var opened bool + origOpenBrowser := openBrowserFunc + openBrowserFunc = func(rawURL string) error { + opened = true + return nil + } + t.Cleanup(func() { openBrowserFunc = origOpenBrowser }) + + var retryCalled bool + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + retryCalled = true + return executor.Result{Response: map[string]any{"ok": true}}, nil + }, + } + + runner := &runtimeRunner{ + fallback: mock, + globalFlags: &GlobalFlags{Format: "table"}, + } + raw := `{"code":"AGENT_CODE_NOT_EXISTS","data":{"desc":"test auth","flowId":"flow-approved","uri":"https://example.com/pat","clientId":"test-client-id"}}` + + var buf bytes.Buffer + _, err := handlePatAuthCheck(context.Background(), runner, executor.Invocation{ + CanonicalProduct: "test", + Tool: "test_tool", + }, &apperrors.PATError{RawJSON: raw}, configDir, &buf) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !retryCalled { + t.Fatal("expected retry to still happen in non-json mode") + } + if opened { + t.Fatal("browser should not open when policy disables it") + } + if !strings.Contains(buf.String(), "需要 PAT 授权") { + t.Fatalf("expected human-readable PAT output, got %q", buf.String()) + } +} + +func TestRetryWithPatAuthRetry_JSONModeReturnsStructuredPATError(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") + configDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", configDir) + if _, err := pat.SetBrowserPolicy(configDir, "", false); err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + t.Fatal("runner should not be called in scope json mode") + return executor.Result{}, nil + }, + } + + runner := &runtimeRunner{ + fallback: mock, + globalFlags: &GlobalFlags{Format: "json"}, + } + scopeErr := &PatScopeError{ + OriginalError: "missing required scope(s): mail:send", + Identity: "user", + ErrorType: "missing_scope", + Message: "missing required scope(s): mail:send", + Hint: "run `dws auth login --scope \"mail:send\"` to authorize the missing scope", + MissingScope: "mail:send", + } + + var buf bytes.Buffer + _, err := retryWithPatAuthRetry(context.Background(), runner, executor.Invocation{}, scopeErr, configDir, &buf) + if err == nil { + t.Fatal("expected PATError") + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output in json scope mode, got %q", got) + } + patOut, ok := err.(*apperrors.PATError) + if !ok { + t.Fatalf("expected *PATError, got %T: %v", err, err) + } + var payload map[string]any + if err := json.Unmarshal([]byte(patOut.RawJSON), &payload); err != nil { + t.Fatalf("json.Unmarshal(scope PAT payload) error = %v\nraw=%s", err, patOut.RawJSON) + } + data, _ := payload["data"].(map[string]any) + if got, ok := data["openBrowser"].(bool); !ok || got { + t.Fatalf("data.openBrowser = %#v, want false", data["openBrowser"]) + } + if _, ok := data["hostControl"]; ok { + t.Fatalf("unexpected data.hostControl in CLI-owned json scope mode: %s", patOut.RawJSON) + } +} + +// TestEnrichPATErrorForHostControl_SingleLineOutput locks in the wire +// invariant: the enriched host-controlled PAT payload must be single-line +// (no embedded newlines, no indentation), so stderr-line-scanning hosts +// stay correct. Regression guard against accidental reintroduction of +// json.MarshalIndent. +func TestEnrichPATErrorForHostControl_SingleLineOutput(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "agt-sales") + t.Setenv("DINGTALK_AGENT", "sales-copilot") + + raw := `{"success":false,"code":"PAT_LOW_RISK_NO_PERMISSION","data":{"flowId":"flow-1","desc":"授权","callbacks":["cb1","cb2"]}}` + out := enrichPATErrorForHostControl(raw) + + if strings.Contains(out, "\n") { + t.Fatalf("enrichPATErrorForHostControl output must be single-line, got embedded newline:\n%s", out) + } + if strings.HasPrefix(out, " ") || strings.HasPrefix(out, "\t") { + t.Fatalf("enrichPATErrorForHostControl output must not be indented, got leading whitespace: %q", out) + } + var parsed map[string]any + if err := json.Unmarshal([]byte(out), &parsed); err != nil { + t.Fatalf("single-line output must round-trip via json.Unmarshal: %v\nraw=%s", err, out) + } + data, _ := parsed["data"].(map[string]any) + hostControl, _ := data["hostControl"].(map[string]any) + if hostControl == nil { + t.Fatalf("expected data.hostControl injection, got: %s", out) + } + if got, _ := hostControl["callbackOwner"].(string); got != "host" { + t.Fatalf("hostControl.callbackOwner = %q, want host", got) + } + if _, ok := data["callbacks"]; ok { + t.Fatalf("expected callbacks to be stripped in host-owned contract, got: %v", data["callbacks"]) + } +} + +// TestBuildPATScopeHostJSON_SingleLineOutput mirrors the above regression +// for the scope-error branch (PAT_SCOPE_AUTH_REQUIRED emission). +func TestBuildPATScopeHostJSON_SingleLineOutput(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "agt-support") + t.Setenv("DINGTALK_AGENT", "customer-support") + + scopeErr := &PatScopeError{ + OriginalError: "missing required scope(s): mail:send", + Identity: "user", + ErrorType: "missing_scope", + Message: "missing required scope(s): mail:send", + Hint: "run `dws auth login --scope \"mail:send\"` to authorize", + MissingScope: "mail:send", + } + out := buildPATScopeJSON(scopeErr, true) + + if strings.Contains(out, "\n") { + t.Fatalf("buildPATScopeJSON(host) output must be single-line, got embedded newline:\n%s", out) + } + if strings.HasPrefix(out, " ") || strings.HasPrefix(out, "\t") { + t.Fatalf("buildPATScopeJSON(host) output must not be indented, got leading whitespace: %q", out) + } + var parsed map[string]any + if err := json.Unmarshal([]byte(out), &parsed); err != nil { + t.Fatalf("single-line output must round-trip via json.Unmarshal: %v\nraw=%s", err, out) + } + if code, _ := parsed["code"].(string); code != "PAT_SCOPE_AUTH_REQUIRED" { + t.Errorf("code = %q, want PAT_SCOPE_AUTH_REQUIRED", code) + } +} + +func TestRetryWithPatAuthRetry_HostControlledReturnsJSON(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "agt-support") + t.Setenv("DINGTALK_AGENT", "customer-support") + + scopeErr := &PatScopeError{ + OriginalError: "missing required scope(s): mail:send", + Identity: "user", + ErrorType: "missing_scope", + Message: "missing required scope(s): mail:send", + Hint: "run `dws auth login --scope \"mail:send\"` to authorize the missing scope", + MissingScope: "mail:send", + } + + mock := &mockRunner{ + runFunc: func(ctx context.Context, inv executor.Invocation) (executor.Result, error) { + t.Fatal("runner should not be called in host-controlled scope mode") + return executor.Result{}, nil + }, + } + + var buf bytes.Buffer + _, err := retryWithPatAuthRetry(context.Background(), mock, executor.Invocation{}, scopeErr, t.TempDir(), &buf) + if err == nil { + t.Fatal("expected PATError") + } + if got := strings.TrimSpace(buf.String()); got != "" { + t.Fatalf("expected no human-readable output, got %q", got) + } + patErr, ok := err.(*apperrors.PATError) + if !ok { + t.Fatalf("expected *PATError, got %T: %v", err, err) + } + var payload map[string]any + if err := json.Unmarshal([]byte(patErr.RawJSON), &payload); err != nil { + t.Fatalf("json.Unmarshal(scope host payload) error = %v\nraw=%s", err, patErr.RawJSON) + } + if got, _ := payload["code"].(string); got != "PAT_SCOPE_AUTH_REQUIRED" { + t.Fatalf("code = %q, want PAT_SCOPE_AUTH_REQUIRED", got) + } + data, _ := payload["data"].(map[string]any) + if got, _ := data["missingScope"].(string); got != "mail:send" { + t.Fatalf("missingScope = %q, want mail:send", got) + } + hostControl, _ := data["hostControl"].(map[string]any) + if got, _ := hostControl["callbackOwner"].(string); got != "host" { + t.Fatalf("hostControl.callbackOwner = %q, want host", got) + } + if _, ok := data["callbacks"]; ok { + t.Fatalf("unexpected callbacks contract in PAT scope host payload: %#v", data["callbacks"]) + } + if strings.Contains(patErr.RawJSON, `"pat","callback"`) { + t.Fatalf("scope host payload should not advertise dws pat callback argv: %s", patErr.RawJSON) + } } func TestHandlePatAuthCheck_OpensOpaqueURIWithoutRebuild(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, "") server, configDir := setupHandlePATServer(t, "APPROVED", "test-auth-code") defer server.Close() diff --git a/internal/app/pat_hostcontrol_wire.go b/internal/app/pat_hostcontrol_wire.go new file mode 100644 index 00000000..0c8f7872 --- /dev/null +++ b/internal/app/pat_hostcontrol_wire.go @@ -0,0 +1,62 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + authpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/auth" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pat" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +// init wires the PAT classifier's hostControl injection hook. This +// guarantees cleanPATJSON emits data.hostControl in host-owned mode +// regardless of whether the PAT error was surfaced via the active retry +// path or the passive classifier path. +// +// Decision rule: +// - Host-owned is triggered iff DINGTALK_DWS_AGENTCODE is non-empty. +// - When triggered, `clawType` in the emitted hostControl block MUST +// be the exact value the CLI actually injects on the wire into the +// `claw-type` HTTP header. The open-source build pins that to +// edition.DefaultOSSClawType ("openClaw") unconditionally — there +// is no per-spawn env override. +// - When DINGTALK_DWS_AGENTCODE is empty the provider returns "" so +// HostControlBlock yields nil and no hostControl block is emitted. +func init() { + apperrors.SetHostControlProvider(hostControlProviderFromEnv) + apperrors.SetPATOpenBrowserProvider(func() bool { + return pat.EffectiveOpenBrowser(defaultConfigDir()) + }) +} + +func hostControlProviderFromEnv() string { + if !authpkg.HostOwnsPATFlow() { + return "" + } + return effectiveClawType() +} + +// effectiveClawType returns the literal value that MergeHeaders will +// inject into outbound `claw-type` headers. Going through the edition +// hook (instead of a hard-coded constant) keeps this site correct for +// downstream editions that override MergeHeaders. +func effectiveClawType() string { + if h := edition.Get(); h != nil && h.MergeHeaders != nil { + if v, ok := h.MergeHeaders(map[string]string{})["claw-type"]; ok && v != "" { + return v + } + } + return edition.DefaultOSSClawType +} diff --git a/internal/app/runner.go b/internal/app/runner.go index 89cd852a..8b522353 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -59,7 +59,7 @@ func init() { configmeta.Register(configmeta.ConfigItem{ Name: "DINGTALK_AGENT", Category: configmeta.CategoryExternal, - Description: "MCP 请求 x-dingtalk-agent 头", + Description: "业务 Agent 名称；仅用于 x-dingtalk-agent 请求头，与 claw-type/host-owned PAT 判定无关", }) configmeta.Register(configmeta.ConfigItem{ Name: "DINGTALK_TRACE_ID", @@ -93,6 +93,27 @@ const ( envDWSChannel = "DWS_CHANNEL" ) +// hostOwnedPATDecisionOnce ensures the host-owned PAT decision is logged at +// most once per CLI process. The log line is emitted at Debug level so +// `--debug` (or `--verbose`) surfaces it on stderr; the file logger at +// ~/.dws/logs/dws.log captures it unconditionally at DEBUG. It records +// ONLY the derived booleans — never the env value, token, client-id or +// flow-id — so logs remain safe to attach to issues. +var hostOwnedPATDecisionOnce sync.Once + +// logHostOwnedPATDecisionOnce emits the single-shot debug trace. It is +// called lazily from the runtime Run path (which executes AFTER +// PersistentPreRunE has applied --debug / --verbose via configureLogLevel) +// so the line actually surfaces when the user asks for it. +func logHostOwnedPATDecisionOnce() { + hostOwnedPATDecisionOnce.Do(func() { + slog.Debug("runtime.host_owned_pat", + "hostOwned", authpkg.HostOwnsPATFlow(), + "agentCodeEnvPresent", os.Getenv(authpkg.AgentCodeEnv) != "", + ) + }) +} + func newCommandRunnerWithFlags(loader cli.CatalogLoader, flags *GlobalFlags) executor.Runner { // Ensure DWS_CLIENT_ID env is populated from persisted config before // resolveIdentityHeaders reads it. This covers fresh-process cold starts @@ -132,6 +153,12 @@ type runtimeRunner struct { } func (r *runtimeRunner) Run(ctx context.Context, invocation executor.Invocation) (executor.Result, error) { + // Emit the one-shot host-owned PAT decision log. Placed here (not in + // the constructor) so it fires AFTER PersistentPreRunE has configured + // slog level per --debug / --verbose. The Once guard makes repeat + // invocations within the same process free. + logHostOwnedPATDecisionOnce() + if r.loader == nil || r.transport == nil { return r.fallback.Run(ctx, invocation) } @@ -610,11 +637,17 @@ func resolveIdentityHeaders() map[string]string { } // Inject environment variable based headers for MCP gateway tracking. + // DINGTALK_AGENT, if set by the caller, is forwarded verbatim as the + // x-dingtalk-agent header. It does NOT influence claw-type (which the + // open-source edition pins to edition.DefaultOSSClawType via the + // MergeHeaders hook below) and it does NOT influence the host-owned + // PAT decision (driven solely by DINGTALK_DWS_AGENTCODE). envHeaders := map[string]string{ - "x-dingtalk-agent": os.Getenv(envDingtalkAgent), - "x-dingtalk-trace-id": os.Getenv(envDingtalkTraceID), - "x-dingtalk-session-id": os.Getenv(envDingtalkSessionID), - "x-dingtalk-message-id": os.Getenv(envDingtalkMessageID), + "x-dingtalk-agent": os.Getenv(envDingtalkAgent), + "x-dingtalk-dws-agent-code": strings.TrimSpace(os.Getenv(authpkg.AgentCodeEnv)), + "x-dingtalk-trace-id": os.Getenv(envDingtalkTraceID), + "x-dingtalk-session-id": os.Getenv(envDingtalkSessionID), + "x-dingtalk-message-id": os.Getenv(envDingtalkMessageID), } for k, v := range envHeaders { if v != "" { @@ -622,7 +655,8 @@ func resolveIdentityHeaders() map[string]string { } } - // Inject third-party channel headers + // Inject third-party channel headers. DWS_CHANNEL is forwarded as the + // upstream channelCode. if v := os.Getenv(envDWSChannel); v != "" { headers["x-dws-channel"] = v } diff --git a/internal/app/runner_test.go b/internal/app/runner_test.go index 3f0b4cc0..6e77eb79 100644 --- a/internal/app/runner_test.go +++ b/internal/app/runner_test.go @@ -25,6 +25,7 @@ import ( "strings" "testing" + authpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/auth" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cli" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/keychain" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" @@ -313,6 +314,16 @@ func TestRuntimeRunnerInjectsAuthTokenFromFlag(t *testing.T) { } } +func TestResolveIdentityHeadersForwardsAgentCode(t *testing.T) { + setupRuntimeCommandTest(t) + t.Setenv(authpkg.AgentCodeEnv, " cursor ") + + headers := resolveIdentityHeaders() + if got := headers["x-dingtalk-dws-agent-code"]; got != "cursor" { + t.Fatalf("x-dingtalk-dws-agent-code = %q, want cursor", got) + } +} + // TestRuntimeRunnerRejectsUnauthenticatedRequest verifies that requests without // a valid token are rejected with a clear error before making any network call. func TestRuntimeRunnerRejectsUnauthenticatedRequest(t *testing.T) { diff --git a/internal/auth/channel.go b/internal/auth/channel.go new file mode 100644 index 00000000..e9c1f49d --- /dev/null +++ b/internal/auth/channel.go @@ -0,0 +1,37 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package auth + +import ( + "os" + "strings" +) + +const ( + // AgentCodeEnv is the sole per-spawn environment variable the host injects + // to declare "this process is driven by a third-party Agent host, render + // authorization UI yourselves". + AgentCodeEnv = "DINGTALK_DWS_AGENTCODE" +) + +// HostOwnsPATFlow reports whether the current process is running under a +// third-party Agent host that will render the PAT authorization card +// itself. The sole trigger is AgentCodeEnv (DINGTALK_DWS_AGENTCODE) being +// non-empty. The CLI deliberately does not consult any other signal +// (DINGTALK_AGENT / DWS_CHANNEL / the wire claw-type header) for this +// decision so that server-side routing tags and the host-owned UI contract +// remain independent concerns. +func HostOwnsPATFlow() bool { + return strings.TrimSpace(os.Getenv(AgentCodeEnv)) != "" +} diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 92585a24..56cfe6a4 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -65,6 +65,10 @@ func (e *Error) Unwrap() error { type Option func(*Error) // ExitCode returns the documented process exit code for the error category. +// exit=4 is reserved exclusively for PATError (see internal/errors/pat.go +// ExitCodePermission and the exit-code table in docs/reference.md); +// Discovery therefore uses 6 so hosts can tell "catalog lookup broke" +// apart from "PAT permission insufficient". func (e *Error) ExitCode() int { switch e.Category { case CategoryAPI: @@ -74,7 +78,7 @@ func (e *Error) ExitCode() int { case CategoryValidation: return 3 case CategoryDiscovery: - return 4 + return 6 default: return 5 } diff --git a/internal/errors/errors_extra_test.go b/internal/errors/errors_extra_test.go index d7a1ab77..249dd54a 100644 --- a/internal/errors/errors_extra_test.go +++ b/internal/errors/errors_extra_test.go @@ -178,7 +178,7 @@ func TestExitCode_AllCategories(t *testing.T) { {CategoryAPI, 1}, {CategoryAuth, 2}, {CategoryValidation, 3}, - {CategoryDiscovery, 4}, + {CategoryDiscovery, 6}, {CategoryInternal, 5}, } for _, tt := range tests { diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go index 2044a61d..293480f4 100644 --- a/internal/errors/errors_test.go +++ b/internal/errors/errors_test.go @@ -29,7 +29,7 @@ func TestExitCodeByCategory(t *testing.T) { {err: NewAPI("api"), want: 1}, {err: NewAuth("auth"), want: 2}, {err: NewValidation("validation"), want: 3}, - {err: NewDiscovery("discovery"), want: 4}, + {err: NewDiscovery("discovery"), want: 6}, {err: NewInternal("internal"), want: 5}, {err: stderrors.New("plain"), want: 5}, } diff --git a/internal/errors/pat.go b/internal/errors/pat.go index b355f69d..92d8156a 100644 --- a/internal/errors/pat.go +++ b/internal/errors/pat.go @@ -18,15 +18,90 @@ import ( stderrors "errors" "fmt" "strings" + "sync" ) +// hostControlProvider returns the host-owned clawType for the current +// process, or empty string when CLI is in default (CLI-owned) mode. +// Injected lazily via SetHostControlProvider to avoid an +// internal/errors → internal/auth import cycle. +// +// Access is serialized by hostControlMu so that tests can swap the provider +// without triggering the race detector against parallel classifier callers. +var ( + hostControlMu sync.RWMutex + hostControlProvider func() string + patBrowserMu sync.RWMutex + patBrowserProvider func() bool +) + +// SetHostControlProvider wires up the classifier's hostControl injection. +// It MUST be called once during CLI bootstrap (e.g. from internal/app +// init()) so that the first cleanPATJSON call observes a valid provider. +// Passing nil disables injection (useful for isolated tests). +func SetHostControlProvider(fn func() string) { + hostControlMu.Lock() + defer hostControlMu.Unlock() + hostControlProvider = fn +} + +// SetPATOpenBrowserProvider wires the PAT JSON serializer to the current +// browser policy. Passing nil restores the open-source fallback (true). +func SetPATOpenBrowserProvider(fn func() bool) { + patBrowserMu.Lock() + defer patBrowserMu.Unlock() + patBrowserProvider = fn +} + +// PATOpenBrowserValue returns the effective browser-open recommendation to +// embed in PAT JSON payloads. The open-source fallback is true to preserve +// historical behavior when no provider is wired. +func PATOpenBrowserValue() bool { + patBrowserMu.RLock() + provider := patBrowserProvider + patBrowserMu.RUnlock() + if provider == nil { + return true + } + return provider() +} + +// HostControlBlock returns the canonical hostControl map injected into +// PAT stderr JSON when the CLI is operating in host-owned mode, or nil +// when it is not. The returned map is safe for the caller to mutate +// because a new map is constructed on each call. +// +// callbackOwner is kept as a legacy compatibility key for hosts that adopted +// it before the contract converged on the hostControl single injection point. +func HostControlBlock() map[string]any { + hostControlMu.RLock() + provider := hostControlProvider + hostControlMu.RUnlock() + if provider == nil { + return nil + } + claw := provider() + if claw == "" { + return nil + } + return map[string]any{ + "clawType": claw, + "callbackOwner": "host", + "mode": "host", + "pollingOwner": "host", + "retryOwner": "host", + } +} + // ExitCodePermission is the process exit code for PAT authorisation failures. const ExitCodePermission = 4 // PATError represents a PAT (Personal Action Token) authorization failure // that should be passed through to stderr as raw JSON without any CLI-layer -// wrapping. The host application (e.g. RewindDesktop) parses the JSON to -// display its own authorisation UI. +// wrapping. The host application parses the JSON to display its own +// authorization UI. The wire schema is fixed: a single-line, directly +// json.Unmarshal-able payload of the form +// {"success":false,"code":,"data":{...}}. // // When the payload includes data.uri, that URL is the authoritative // server-provided authorization link. Hosts must treat it as opaque and open @@ -54,9 +129,21 @@ var patNoPermissionCodes = map[string]bool{ } // patAuthRequiredCodes are error codes that trigger the PAT authorization -// flow (e.g. the server auto-created a CLI app and returned auth details). +// flow (e.g. the server auto-created a CLI app and returned auth details, +// or the caller's OAuth token lacks a scope that must be re-acquired via +// `dws auth login --scope `). +// +// Keep keys in alphabetical order so diffs are stable. Both codes below are +// part of the frozen PAT-family selector and MUST be surfaced as *PATError +// (exit=4) so hosts can act on them: +// - AGENT_CODE_NOT_EXISTS: data.agentCode tells the host which agent +// registration is missing. +// - PAT_SCOPE_AUTH_REQUIRED: data.missingScope tells the host which +// OAuth scope to re-acquire via +// `dws auth login --scope `. var patAuthRequiredCodes = map[string]bool{ - "AGENT_CODE_NOT_EXISTS": true, + "AGENT_CODE_NOT_EXISTS": true, + "PAT_SCOPE_AUTH_REQUIRED": true, } // IsPATError reports whether err is a *PATError. @@ -70,6 +157,35 @@ func IsPATNoPermissionCode(code string) bool { return patNoPermissionCodes[code] } +// errCodeKeys is the canonical priority order in which we look up +// upstream error code fields. Servers historically rotated between camel +// and snake case; we accept all three and pick the first that resolves to +// a recognised value. +var errCodeKeys = []string{"code", "errorCode", "error_code"} + +// lookupCodeIn returns the first value in body[errCodeKeys] that is a +// non-empty string AND is a member of accept. Used by the PAT and DWS +// gateway classifiers, which differ only in their accept-set. +func lookupCodeIn(body map[string]any, accept map[string]bool) (string, bool) { + for _, key := range errCodeKeys { + if code, ok := body[key].(string); ok && accept[code] { + return code, true + } + } + return "", false +} + +// getPATErrorCode extracts any PAT-intercept code from a map. PAT +// intercepts include both permission denials and auth-required selectors: +// callers on the text/tool-result path must preserve both families as +// *PATError so exit=4 + raw stderr JSON survives all the way to the host/CLI. +func getPATErrorCode(body map[string]any) (string, bool) { + if code, ok := lookupCodeIn(body, patNoPermissionCodes); ok { + return code, true + } + return lookupCodeIn(body, patAuthRequiredCodes) +} + // ---- DWS gateway auth errors (shared between PAT & general auth) ---------- // dwsGatewayErrors is the set of DWS gateway-level auth error codes. @@ -78,20 +194,18 @@ var dwsGatewayErrors = map[string]bool{ "DWS_AUTH_SERVICE_FAILED": true, } -// getDWSGatewayErrorCode extracts a DWS gateway error code from errBody -// (supports both errorCode and error_code field names). +// getDWSGatewayErrorCode extracts a DWS gateway error code from errBody. func getDWSGatewayErrorCode(errBody map[string]any) (string, bool) { - for _, key := range []string{"errorCode", "error_code"} { - if code, ok := errBody[key].(string); ok && dwsGatewayErrors[code] { - return code, true - } - } - return "", false + return lookupCodeIn(errBody, dwsGatewayErrors) } // isNotLoggedInError checks if the error body indicates missing authentication. func isNotLoggedInError(body map[string]any) bool { - if errMsg, ok := body["error"].(string); ok { + for _, key := range []string{"error", "message", "errorMsg"} { + errMsg, ok := body[key].(string) + if !ok { + continue + } if strings.Contains(errMsg, "Missing service_id or access_key") { return true } @@ -129,10 +243,8 @@ func ClassifyToolResultContent(content map[string]any) error { WithHint(authExpiredHint()), ) } - for _, key := range []string{"code", "errorCode"} { - if code, ok := content[key].(string); ok && patNoPermissionCodes[code] { - return &PATError{RawJSON: cleanPATJSON(content, code)} - } + if code, ok := getPATErrorCode(content); ok { + return &PATError{RawJSON: cleanPATJSON(content, code)} } return nil } @@ -163,10 +275,8 @@ func ClassifyMCPResponseText(text string) error { ) } - for _, key := range []string{"code", "errorCode"} { - if code, ok := body[key].(string); ok && patNoPermissionCodes[code] { - return &PATError{RawJSON: cleanPATJSON(body, code)} - } + if code, ok := getPATErrorCode(body); ok { + return &PATError{RawJSON: cleanPATJSON(body, code)} } if isBusinessError(body) { @@ -219,6 +329,33 @@ var patTopLevelStrip = map[string]bool{ "message": true, "error": true, "trace_id": true, "class": true, } +// ApplyHostMutations writes the two stderr-JSON fields the host integration +// contract requires onto out["data"]: +// - data.hostControl: present iff the CLI is in host-owned mode (i.e. +// HostControlBlock returns non-nil); legacy data.callbacks is stripped +// in the same pass so passive classifier and active retry paths stay +// byte-for-byte aligned. +// - data.openBrowser: always present; reflects the user's PAT browser +// policy. +// +// Centralizing the two writes here is the single-injection invariant — +// any caller that produces a PAT-shaped stderr payload (cleanPATJSON, +// active-retry enrichers, scope-required builders) MUST go through this +// function instead of writing the fields directly. out["data"] is +// promoted to map[string]any if missing or of the wrong type. +func ApplyHostMutations(out map[string]any) { + data, ok := out["data"].(map[string]any) + if !ok || data == nil { + data = map[string]any{} + out["data"] = data + } + if block := HostControlBlock(); block != nil { + delete(data, "callbacks") + data["hostControl"] = block + } + data["openBrowser"] = PATOpenBrowserValue() +} + func cleanPATJSON(body map[string]any, code string) string { out := map[string]any{ "success": false, @@ -240,7 +377,12 @@ func cleanPATJSON(body map[string]any, code string) string { out["data"] = stripClassFields(fallback) } } - b, err := json.MarshalIndent(out, "", " ") + ApplyHostMutations(out) + + // stderr JSON MUST be a single-line, directly json.Unmarshal-able + // payload — pretty-printing would break naïve host parsers that read + // stderr line-by-line and fail on leading whitespace. + b, err := json.Marshal(out) if err != nil { return fmt.Sprintf(`{"success":false,"code":"%s"}`, code) } @@ -252,15 +394,11 @@ func cleanPATJSON(body map[string]any, code string) string { // framework (ClassifyPatAuthCheck / AsPatAuthCheckError). // ClassifyPatAuthCheck is the open-source fallback that checks a tool-call -// Content map for PAT permission codes and auth-required codes. Returns a +// Content map for PAT permission codes and auth-required codes. Returns a // non-nil *PATError when the content carries a recognised PAT/auth error. func ClassifyPatAuthCheck(content map[string]any) *PATError { - for _, key := range []string{"code", "errorCode"} { - if code, ok := content[key].(string); ok { - if patNoPermissionCodes[code] || patAuthRequiredCodes[code] { - return &PATError{RawJSON: cleanPATJSON(content, code)} - } - } + if code, ok := getPATErrorCode(content); ok { + return &PATError{RawJSON: cleanPATJSON(content, code)} } return nil } diff --git a/internal/errors/pat_test.go b/internal/errors/pat_test.go index ec2204f5..ede23dce 100644 --- a/internal/errors/pat_test.go +++ b/internal/errors/pat_test.go @@ -144,8 +144,17 @@ func TestIsNotLoggedInError_False(t *testing.T) { func TestIsNotLoggedInError_NoErrorField(t *testing.T) { t.Parallel() body := map[string]any{"message": "Missing service_id or access_key"} - if isNotLoggedInError(body) { - t.Fatal("expected false when error field is absent") + if !isNotLoggedInError(body) { + t.Fatal("expected true when equivalent auth message is present in message") + } +} + +func TestGetDWSGatewayErrorCode_CodeField(t *testing.T) { + t.Parallel() + body := map[string]any{"code": "DWS_SERVICE_UNAUTHORIZED"} + code, ok := getDWSGatewayErrorCode(body) + if !ok || code != "DWS_SERVICE_UNAUTHORIZED" { + t.Fatalf("getDWSGatewayErrorCode() = (%q, %t), want DWS_SERVICE_UNAUTHORIZED, true", code, ok) } } @@ -235,6 +244,44 @@ func TestClassifyToolResultContent_PATPermission(t *testing.T) { } } +func TestClassifyToolResultContent_PATPermissionLegacyErrorCode(t *testing.T) { + t.Parallel() + content := map[string]any{ + "error_code": "PAT_LOW_RISK_NO_PERMISSION", + "data": map[string]any{"desc": "需要授权"}, + } + err := ClassifyToolResultContent(content) + if err == nil { + t.Fatal("expected non-nil error for legacy error_code PAT permission") + } + var patErr *PATError + if !stderrors.As(err, &patErr) { + t.Fatalf("expected *PATError, got %T", err) + } + if !strings.Contains(patErr.RawJSON, "PAT_LOW_RISK_NO_PERMISSION") { + t.Errorf("RawJSON should contain PAT_LOW_RISK_NO_PERMISSION, got: %s", patErr.RawJSON) + } +} + +func TestClassifyToolResultContent_PATAuthRequired(t *testing.T) { + t.Parallel() + content := map[string]any{ + "errorCode": "AGENT_CODE_NOT_EXISTS", + "data": map[string]any{"agentCode": "agt-missing"}, + } + err := ClassifyToolResultContent(content) + if err == nil { + t.Fatal("expected non-nil error for PAT auth-required selector") + } + var patErr *PATError + if !stderrors.As(err, &patErr) { + t.Fatalf("expected *PATError, got %T", err) + } + if !strings.Contains(patErr.RawJSON, "AGENT_CODE_NOT_EXISTS") { + t.Errorf("RawJSON should contain AGENT_CODE_NOT_EXISTS, got: %s", patErr.RawJSON) + } +} + func TestClassifyToolResultContent_NoError(t *testing.T) { t.Parallel() content := map[string]any{"success": true, "data": "ok"} @@ -295,6 +342,41 @@ func TestClassifyMCPResponseText_PATPermission(t *testing.T) { } } +func TestClassifyMCPResponseText_PATPermissionLegacyErrorCode(t *testing.T) { + t.Parallel() + text := `{"error_code":"PAT_MEDIUM_RISK_NO_PERMISSION","data":{"desc":"legacy"}}` + err := ClassifyMCPResponseText(text) + if err == nil { + t.Fatal("expected non-nil error") + } + var patErr *PATError + if !stderrors.As(err, &patErr) { + t.Fatalf("expected *PATError, got %T", err) + } + if !strings.Contains(patErr.RawJSON, "PAT_MEDIUM_RISK_NO_PERMISSION") { + t.Errorf("RawJSON should contain legacy code, got: %s", patErr.RawJSON) + } +} + +func TestClassifyMCPResponseText_PATAuthRequired(t *testing.T) { + t.Parallel() + text := `{"success":false,"code":"PAT_SCOPE_AUTH_REQUIRED","data":{"missingScope":"mail:send"}}` + err := ClassifyMCPResponseText(text) + if err == nil { + t.Fatal("expected non-nil error") + } + var patErr *PATError + if !stderrors.As(err, &patErr) { + t.Fatalf("expected *PATError, got %T", err) + } + if !strings.Contains(patErr.RawJSON, "PAT_SCOPE_AUTH_REQUIRED") { + t.Errorf("RawJSON should contain PAT_SCOPE_AUTH_REQUIRED, got: %s", patErr.RawJSON) + } + if !strings.Contains(patErr.RawJSON, "missingScope") { + t.Errorf("RawJSON should preserve missingScope, got: %s", patErr.RawJSON) + } +} + func TestClassifyMCPResponseText_BusinessError(t *testing.T) { t.Parallel() text := `{"success":false,"errorMsg":"搜索内容不能为空"}` @@ -346,6 +428,18 @@ func TestClassifyPatAuthCheck_PATNoPermission(t *testing.T) { } } +func TestClassifyPatAuthCheck_LegacyErrorCode(t *testing.T) { + t.Parallel() + content := map[string]any{"error_code": "PAT_HIGH_RISK_NO_PERMISSION", "data": map[string]any{"flowId": "f1"}} + patErr := ClassifyPatAuthCheck(content) + if patErr == nil { + t.Fatal("expected non-nil *PATError for legacy error_code") + } + if !strings.Contains(patErr.RawJSON, "PAT_HIGH_RISK_NO_PERMISSION") { + t.Errorf("RawJSON should contain PAT_HIGH_RISK_NO_PERMISSION, got: %s", patErr.RawJSON) + } +} + func TestClassifyPatAuthCheck_AgentCodeNotExists(t *testing.T) { t.Parallel() content := map[string]any{"errorCode": "AGENT_CODE_NOT_EXISTS", "data": map[string]any{"clientId": "c1"}} @@ -358,6 +452,43 @@ func TestClassifyPatAuthCheck_AgentCodeNotExists(t *testing.T) { } } +// TestClassifyPatAuthCheck_scope_auth_required pins the PAT_SCOPE_AUTH_REQUIRED +// selector (part of the frozen PAT-family enum; see patAuthRequiredCodes in +// internal/errors/pat.go) as a PATError with exit=4 so hosts can kick the +// `dws auth login --scope ` branch. +func TestClassifyPatAuthCheck_scope_auth_required(t *testing.T) { + t.Parallel() + content := map[string]any{ + "success": false, + "code": "PAT_SCOPE_AUTH_REQUIRED", + "data": map[string]any{"missingScope": "mail:send"}, + } + patErr := ClassifyPatAuthCheck(content) + if patErr == nil { + t.Fatal("expected non-nil *PATError for PAT_SCOPE_AUTH_REQUIRED") + } + + // Error value MUST satisfy the ExitCoder contract (exit=4) so the + // process exits with the PAT Frozen code regardless of wrapping. + var ec interface{ ExitCode() int } = patErr + if ec.ExitCode() != ExitCodePermission { + t.Errorf("ExitCode() = %d, want %d", ec.ExitCode(), ExitCodePermission) + } + + // Host-visible RawStderr must carry the selector and, crucially, the + // missingScope field that drives `dws auth login --scope `. + raw := patErr.RawStderr() + if !strings.Contains(raw, "PAT_SCOPE_AUTH_REQUIRED") { + t.Errorf("RawStderr missing selector, got: %s", raw) + } + if !strings.Contains(raw, "missingScope") { + t.Errorf("RawStderr missing missingScope field, got: %s", raw) + } + if !strings.Contains(raw, "mail:send") { + t.Errorf("RawStderr missing missingScope value, got: %s", raw) + } +} + func TestClassifyPatAuthCheck_NoMatch(t *testing.T) { t.Parallel() content := map[string]any{"code": "SOME_BUSINESS_ERROR", "message": "oops"} @@ -424,6 +555,9 @@ func TestCleanPATJSON_WithData(t *testing.T) { if strings.Contains(result, "class") { t.Errorf("expected class field to be stripped, got: %s", result) } + if !strings.Contains(result, `"openBrowser":true`) { + t.Errorf("expected openBrowser default in output, got: %s", result) + } } func TestCleanPATJSON_WithoutData(t *testing.T) { @@ -444,6 +578,142 @@ func TestCleanPATJSON_WithoutData(t *testing.T) { } } +// TestCleanPATJSON_InjectsHostControlWhenClawSet verifies the +// single-injection invariant: when the bootstrap wires a non-empty +// clawType provider, cleanPATJSON MUST emit data.hostControl. +func TestCleanPATJSON_InjectsHostControlWhenClawSet(t *testing.T) { + // Not parallel: mutates the package-level provider. + t.Cleanup(func() { SetHostControlProvider(nil) }) + SetHostControlProvider(func() string { return "my-copilot" }) + + body := map[string]any{ + "success": false, + "code": "PAT_NO_PERMISSION", + "data": map[string]any{ + "desc": "需要授权", + "flowId": "f-1", + "callbacks": []any{"cb1", "cb2"}, + }, + } + raw := cleanPATJSON(body, "PAT_NO_PERMISSION") + + var parsed map[string]any + if err := json.Unmarshal([]byte(raw), &parsed); err != nil { + t.Fatalf("unmarshal cleanPATJSON output: %v\nraw=%s", err, raw) + } + data, ok := parsed["data"].(map[string]any) + if !ok { + t.Fatalf("expected data object, got %T", parsed["data"]) + } + hc, ok := data["hostControl"].(map[string]any) + if !ok { + t.Fatalf("expected data.hostControl to be a map, got %T\nraw=%s", data["hostControl"], raw) + } + if got, _ := hc["clawType"].(string); got != "my-copilot" { + t.Errorf("hostControl.clawType = %q, want %q", got, "my-copilot") + } + if got, _ := hc["callbackOwner"].(string); got != "host" { + t.Errorf("hostControl.callbackOwner = %q, want %q", got, "host") + } + if got, _ := hc["mode"].(string); got != "host" { + t.Errorf("hostControl.mode = %q, want %q", got, "host") + } + if _, ok := data["callbacks"]; ok { + t.Fatalf("cleanPATJSON should strip callbacks in host-owned mode, got: %v", data["callbacks"]) + } +} + +// TestCleanPATJSON_OmitsHostControlByDefault verifies that cleanPATJSON +// does NOT include a hostControl block when the provider is unset or +// returns empty (default CLI-owned mode). +func TestCleanPATJSON_OmitsHostControlByDefault(t *testing.T) { + // Not parallel: reads the package-level provider. + t.Cleanup(func() { SetHostControlProvider(nil) }) + SetHostControlProvider(nil) + + body := map[string]any{ + "success": false, + "code": "PAT_NO_PERMISSION", + "data": map[string]any{ + "desc": "need auth", + }, + } + raw := cleanPATJSON(body, "PAT_NO_PERMISSION") + if strings.Contains(raw, `"hostControl"`) { + t.Fatalf("cleanPATJSON should omit hostControl in default mode, got: %s", raw) + } + + SetHostControlProvider(func() string { return "" }) + raw = cleanPATJSON(body, "PAT_NO_PERMISSION") + if strings.Contains(raw, `"hostControl"`) { + t.Fatalf("cleanPATJSON should omit hostControl when provider returns empty, got: %s", raw) + } +} + +func TestCleanPATJSON_UsesBrowserPolicyProvider(t *testing.T) { + t.Cleanup(func() { + SetHostControlProvider(nil) + SetPATOpenBrowserProvider(nil) + }) + SetHostControlProvider(nil) + SetPATOpenBrowserProvider(func() bool { return false }) + + body := map[string]any{ + "success": false, + "code": "PAT_NO_PERMISSION", + "data": map[string]any{ + "desc": "need auth", + }, + } + raw := cleanPATJSON(body, "PAT_NO_PERMISSION") + + var parsed map[string]any + if err := json.Unmarshal([]byte(raw), &parsed); err != nil { + t.Fatalf("unmarshal cleanPATJSON output: %v\nraw=%s", err, raw) + } + data, _ := parsed["data"].(map[string]any) + if got, ok := data["openBrowser"].(bool); !ok || got { + t.Fatalf("data.openBrowser = %#v, want false", data["openBrowser"]) + } +} + +// TestCleanPATJSON_SingleLineOutput pins down the wire invariant: stderr +// JSON MUST be emitted as a single line (no embedded \n, no pretty-print +// indentation) so that naïve host parsers reading stderr line-by-line stay +// correct. Regression guard against accidental reintroduction of +// json.MarshalIndent. +func TestCleanPATJSON_SingleLineOutput(t *testing.T) { + t.Parallel() + body := map[string]any{ + "success": false, + "code": "PAT_LOW_RISK_NO_PERMISSION", + "data": map[string]any{ + "requiredScopes": []any{"aitable.record:read"}, + "grantOptions": []any{"session", "permanent"}, + "displayName": "读取记录", + "productName": "AI 表格", + }, + } + raw := cleanPATJSON(body, "PAT_LOW_RISK_NO_PERMISSION") + + if strings.Contains(raw, "\n") { + t.Fatalf("cleanPATJSON output must be single-line, got embedded newline:\n%s", raw) + } + if strings.HasPrefix(raw, " ") || strings.HasPrefix(raw, "\t") { + t.Fatalf("cleanPATJSON output must not be indented, got leading whitespace: %q", raw) + } + + // Contract: the payload must remain a directly json.Unmarshal-able + // object, even after the single-line constraint is enforced. + var parsed map[string]any + if err := json.Unmarshal([]byte(raw), &parsed); err != nil { + t.Fatalf("single-line output must round-trip via json.Unmarshal: %v\nraw=%s", err, raw) + } + if code, _ := parsed["code"].(string); code != "PAT_LOW_RISK_NO_PERMISSION" { + t.Errorf("code = %q, want %q", code, "PAT_LOW_RISK_NO_PERMISSION") + } +} + func TestCleanPATJSON_PreservesOpaqueURIVerbatim(t *testing.T) { t.Parallel() rawURI := "https://open-dev.dingtalk.com/fe/old?hash=%23%2FpersonalAuthorization%3FflowId%3D50dff7654b7444e88ced7489b07cce8d%26userCode%3DQ8RY-X6E9#/personalAuthorization?flowId=50dff7654b7444e88ced7489b07cce8d&userCode=Q8RY-X6E9" diff --git a/internal/pat/browser_policy.go b/internal/pat/browser_policy.go new file mode 100644 index 00000000..c6c474b3 --- /dev/null +++ b/internal/pat/browser_policy.go @@ -0,0 +1,228 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pat + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/spf13/cobra" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/output" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +const browserPolicyFile = "pat_policy.json" + +type browserPolicyValue struct { + OpenBrowser bool `json:"openBrowser"` +} + +type BrowserPolicy struct { + Default *browserPolicyValue `json:"default,omitempty"` + Agents map[string]browserPolicyValue `json:"agents,omitempty"` +} + +type BrowserPolicySelection struct { + Scope string `json:"scope"` + AgentCode string `json:"agentCode,omitempty"` + OpenBrowser bool `json:"openBrowser"` + Source string `json:"source"` +} + +func patPolicyPath(configDir string) string { + return filepath.Join(configDir, browserPolicyFile) +} + +func patConfigDir() string { + if envDir := os.Getenv("DWS_CONFIG_DIR"); envDir != "" { + return envDir + } + if fn := edition.Get().ConfigDir; fn != nil { + return fn() + } + homeDir, err := os.UserHomeDir() + if err != nil { + return ".dws" + } + return filepath.Join(homeDir, ".dws") +} + +func LoadBrowserPolicy(configDir string) (*BrowserPolicy, error) { + path := patPolicyPath(configDir) + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return &BrowserPolicy{}, nil + } + return nil, fmt.Errorf("reading PAT browser policy: %w", err) + } + + var policy BrowserPolicy + if err := json.Unmarshal(data, &policy); err != nil { + return nil, fmt.Errorf("parsing PAT browser policy: %w", err) + } + if policy.Agents == nil { + policy.Agents = map[string]browserPolicyValue{} + } + return &policy, nil +} + +func saveBrowserPolicy(configDir string, policy *BrowserPolicy) error { + if policy == nil { + policy = &BrowserPolicy{} + } + if policy.Agents == nil { + policy.Agents = map[string]browserPolicyValue{} + } + data, err := json.MarshalIndent(policy, "", " ") + if err != nil { + return fmt.Errorf("marshaling PAT browser policy: %w", err) + } + data = append(data, '\n') + if err := helpers.AtomicWriteJSON(patPolicyPath(configDir), data); err != nil { + return fmt.Errorf("writing PAT browser policy: %w", err) + } + return nil +} + +func ResolveBrowserPolicy(configDir, explicitAgentCode string) (BrowserPolicySelection, error) { + agentCode, err := resolveAgentCode(explicitAgentCode, false) + if err != nil { + return BrowserPolicySelection{}, err + } + policy, err := LoadBrowserPolicy(configDir) + if err != nil { + return BrowserPolicySelection{}, err + } + + if agentCode != "" { + if entry, ok := policy.Agents[agentCode]; ok { + return BrowserPolicySelection{ + Scope: "agent", + AgentCode: agentCode, + OpenBrowser: entry.OpenBrowser, + Source: "agent", + }, nil + } + } + + if policy.Default != nil { + return BrowserPolicySelection{ + Scope: "default", + OpenBrowser: policy.Default.OpenBrowser, + Source: "default", + }, nil + } + + return BrowserPolicySelection{ + Scope: "builtin_default", + AgentCode: agentCode, + OpenBrowser: true, + Source: "builtin_default", + }, nil +} + +func EffectiveOpenBrowser(configDir string) bool { + selection, err := ResolveBrowserPolicy(configDir, "") + if err != nil { + return true + } + return selection.OpenBrowser +} + +func resolveBrowserPolicyWriteAgentCode(explicitAgentCode string) (string, error) { + agentCode := strings.TrimSpace(explicitAgentCode) + if agentCode == "" { + return "", nil + } + if err := validateAgentCode(agentCode); err != nil { + return "", err + } + return agentCode, nil +} + +func SetBrowserPolicy(configDir, explicitAgentCode string, enabled bool) (BrowserPolicySelection, error) { + agentCode, err := resolveBrowserPolicyWriteAgentCode(explicitAgentCode) + if err != nil { + return BrowserPolicySelection{}, err + } + + policy, err := LoadBrowserPolicy(configDir) + if err != nil { + return BrowserPolicySelection{}, err + } + if policy.Agents == nil { + policy.Agents = map[string]browserPolicyValue{} + } + + if agentCode != "" { + policy.Agents[agentCode] = browserPolicyValue{OpenBrowser: enabled} + if err := saveBrowserPolicy(configDir, policy); err != nil { + return BrowserPolicySelection{}, err + } + return BrowserPolicySelection{ + Scope: "agent", + AgentCode: agentCode, + OpenBrowser: enabled, + Source: "agent", + }, nil + } + + policy.Default = &browserPolicyValue{OpenBrowser: enabled} + if err := saveBrowserPolicy(configDir, policy); err != nil { + return BrowserPolicySelection{}, err + } + return BrowserPolicySelection{ + Scope: "default", + OpenBrowser: enabled, + Source: "default", + }, nil +} + +func newBrowserPolicyCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "browser-policy", + Short: "配置 PAT 授权时是否打开浏览器", + DisableAutoGenTag: true, + RunE: func(cmd *cobra.Command, args []string) error { + if !cmd.Flags().Changed("enabled") { + return fmt.Errorf("--enabled is required") + } + + enabled, err := cmd.Flags().GetBool("enabled") + if err != nil { + return err + } + agentCode, err := cmd.Flags().GetString("agentCode") + if err != nil { + return err + } + + selection, err := SetBrowserPolicy(patConfigDir(), agentCode, enabled) + if err != nil { + return err + } + return output.WriteJSON(cmd.OutOrStdout(), selection) + }, + } + + cmd.Flags().Bool("enabled", false, "PAT 撞墙时是否允许本地打开浏览器") + cmd.Flags().String("agentCode", "", "Agent 唯一标识（可选；不填则写入全局默认策略，不从 env DINGTALK_DWS_AGENTCODE 回退）") + return cmd +} diff --git a/internal/pat/browser_policy_test.go b/internal/pat/browser_policy_test.go new file mode 100644 index 00000000..7819f2c3 --- /dev/null +++ b/internal/pat/browser_policy_test.go @@ -0,0 +1,237 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pat + +import ( + "bytes" + "encoding/json" + "testing" +) + +func TestBrowserPolicy_DefaultRoundTrip(t *testing.T) { + t.Parallel() + + configDir := t.TempDir() + saved, err := SetBrowserPolicy(configDir, "", false) + if err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + if saved.Scope != "default" { + t.Fatalf("saved.Scope = %q, want default", saved.Scope) + } + if saved.OpenBrowser { + t.Fatal("saved.OpenBrowser = true, want false") + } + + loaded, err := ResolveBrowserPolicy(configDir, "") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(default) error = %v", err) + } + if loaded.Scope != "default" { + t.Fatalf("loaded.Scope = %q, want default", loaded.Scope) + } + if loaded.OpenBrowser { + t.Fatal("loaded.OpenBrowser = true, want false") + } +} + +func TestSetBrowserPolicy_EmptyAgentCodeIgnoresEnvAndWritesDefault(t *testing.T) { + t.Setenv(agentCodeEnv, "agt-env") + configDir := t.TempDir() + + saved, err := SetBrowserPolicy(configDir, "", false) + if err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + if saved.Scope != "default" { + t.Fatalf("saved.Scope = %q, want default", saved.Scope) + } + if saved.AgentCode != "" { + t.Fatalf("saved.AgentCode = %q, want empty", saved.AgentCode) + } + + policy, err := LoadBrowserPolicy(configDir) + if err != nil { + t.Fatalf("LoadBrowserPolicy error = %v", err) + } + if policy.Default == nil { + t.Fatal("policy.Default is nil, want default policy") + } + if got := len(policy.Agents); got != 0 { + t.Fatalf("len(policy.Agents) = %d, want 0", got) + } + + loaded, err := ResolveBrowserPolicy(configDir, "") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(default under env) error = %v", err) + } + if loaded.Scope != "default" { + t.Fatalf("loaded.Scope = %q, want default", loaded.Scope) + } + if loaded.AgentCode != "" { + t.Fatalf("loaded.AgentCode = %q, want empty", loaded.AgentCode) + } + if loaded.OpenBrowser { + t.Fatal("loaded.OpenBrowser = true, want false") + } +} + +func TestBrowserPolicy_AgentOverridesDefault(t *testing.T) { + t.Parallel() + + configDir := t.TempDir() + if _, err := SetBrowserPolicy(configDir, "", true); err != nil { + t.Fatalf("SetBrowserPolicy(default) error = %v", err) + } + if _, err := SetBrowserPolicy(configDir, "agt-sales", false); err != nil { + t.Fatalf("SetBrowserPolicy(agent) error = %v", err) + } + + agentLoaded, err := ResolveBrowserPolicy(configDir, "agt-sales") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(agent) error = %v", err) + } + if agentLoaded.Scope != "agent" { + t.Fatalf("agentLoaded.Scope = %q, want agent", agentLoaded.Scope) + } + if agentLoaded.AgentCode != "agt-sales" { + t.Fatalf("agentLoaded.AgentCode = %q, want agt-sales", agentLoaded.AgentCode) + } + if agentLoaded.OpenBrowser { + t.Fatal("agentLoaded.OpenBrowser = true, want false") + } + + defaultLoaded, err := ResolveBrowserPolicy(configDir, "agt-other") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(default fallback) error = %v", err) + } + if defaultLoaded.Scope != "default" { + t.Fatalf("defaultLoaded.Scope = %q, want default", defaultLoaded.Scope) + } + if !defaultLoaded.OpenBrowser { + t.Fatal("defaultLoaded.OpenBrowser = false, want true") + } +} + +func TestResolveBrowserPolicy_EnvFallback(t *testing.T) { + t.Setenv(agentCodeEnv, "agt-env") + configDir := t.TempDir() + if _, err := SetBrowserPolicy(configDir, "agt-env", false); err != nil { + t.Fatalf("SetBrowserPolicy(agent) error = %v", err) + } + + loaded, err := ResolveBrowserPolicy(configDir, "") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(env fallback) error = %v", err) + } + if loaded.Scope != "agent" { + t.Fatalf("loaded.Scope = %q, want agent", loaded.Scope) + } + if loaded.AgentCode != "agt-env" { + t.Fatalf("loaded.AgentCode = %q, want agt-env", loaded.AgentCode) + } + if loaded.OpenBrowser { + t.Fatal("loaded.OpenBrowser = true, want false") + } +} + +func TestResolveBrowserPolicy_FallsBackToOpenSourceDefault(t *testing.T) { + t.Parallel() + + loaded, err := ResolveBrowserPolicy(t.TempDir(), "") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(fallback) error = %v", err) + } + if loaded.Scope != "builtin_default" { + t.Fatalf("loaded.Scope = %q, want builtin_default", loaded.Scope) + } + if !loaded.OpenBrowser { + t.Fatal("loaded.OpenBrowser = false, want true") + } +} + +func TestBrowserPolicyCommand_WritesAgentPolicy(t *testing.T) { + configDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", configDir) + + cmd := newBrowserPolicyCommand() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stdout) + cmd.SetArgs([]string{"--enabled=false", "--agentCode", "agt-command"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("browser-policy Execute() error = %v", err) + } + + var payload map[string]any + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("json.Unmarshal(command output) error = %v\nraw=%s", err, stdout.String()) + } + if got, _ := payload["scope"].(string); got != "agent" { + t.Fatalf("scope = %q, want agent", got) + } + if got, _ := payload["agentCode"].(string); got != "agt-command" { + t.Fatalf("agentCode = %q, want agt-command", got) + } + if got, _ := payload["openBrowser"].(bool); got { + t.Fatal("openBrowser = true, want false") + } + + loaded, err := ResolveBrowserPolicy(configDir, "agt-command") + if err != nil { + t.Fatalf("ResolveBrowserPolicy(agent) error = %v", err) + } + if loaded.OpenBrowser { + t.Fatal("loaded.OpenBrowser = true, want false") + } +} + +func TestBrowserPolicyCommand_NoAgentCodeWritesDefaultEvenWhenEnvSet(t *testing.T) { + configDir := t.TempDir() + t.Setenv("DWS_CONFIG_DIR", configDir) + t.Setenv(agentCodeEnv, "agt-env") + + cmd := newBrowserPolicyCommand() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stdout) + cmd.SetArgs([]string{"--enabled=false"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("browser-policy Execute() error = %v", err) + } + + var payload map[string]any + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("json.Unmarshal(command output) error = %v\nraw=%s", err, stdout.String()) + } + if got, _ := payload["scope"].(string); got != "default" { + t.Fatalf("scope = %q, want default", got) + } + if _, ok := payload["agentCode"]; ok { + t.Fatalf("unexpected agentCode in default policy output: %v", payload["agentCode"]) + } + + policy, err := LoadBrowserPolicy(configDir) + if err != nil { + t.Fatalf("LoadBrowserPolicy error = %v", err) + } + if policy.Default == nil { + t.Fatal("policy.Default is nil, want default policy") + } + if got := len(policy.Agents); got != 0 { + t.Fatalf("len(policy.Agents) = %d, want 0", got) + } +} diff --git a/internal/pat/chmod.go b/internal/pat/chmod.go index 7d75880a..0653a7c3 100644 --- a/internal/pat/chmod.go +++ b/internal/pat/chmod.go @@ -16,8 +16,11 @@ package pat import ( "context" "encoding/json" + stderrors "errors" "fmt" "os" + "regexp" + "strings" "github.com/fatih/color" "github.com/spf13/cobra" @@ -26,13 +29,132 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" ) +// resolveSessionIDFromEnv returns the effective session id from environment +// variables. Resolution order: +// 1. DWS_SESSION_ID (primary, stable env name). +// 2. REWIND_SESSION_ID (compatibility alias; kept only so hosts that +// already inject the legacy trace triple keep working without code +// churn). +// +// When both are set to different non-empty values, DWS_SESSION_ID wins +// silently. We deliberately do NOT log either raw session id value or +// any derived fingerprint: this resolver is invoked by `dws pat chmod` +// session grants, and any stderr / ~/.dws/logs capture of those +// identifiers can land verbatim in attached troubleshooting bundles. +// Hosts that need to detect a mismatch between the two env vars must do +// so on the host side before invoking the CLI. +func resolveSessionIDFromEnv() string { + if dws := os.Getenv("DWS_SESSION_ID"); dws != "" { + return dws + } + return os.Getenv("REWIND_SESSION_ID") +} + +// agentCodeEnv is the canonical (and only) environment variable name +// used as a per-shell fallback for the --agentCode flag on `dws pat *` +// commands. +// +// Why: agent hosts typically set their business agent code once when +// spawning a long-lived shell / sub-process; requiring `--agentCode` on +// every command in that shell forces the host to rewrite every argv. +// Exposing DINGTALK_DWS_AGENTCODE lets the host export the code once and +// let the CLI resolve it on every pat subcommand. The flag always wins +// when both are set so scripted one-offs remain deterministic. +// +// Namespace note: DWS_AGENTCODE / DINGTALK_AGENTCODE / REWIND_AGENTCODE +// are explicitly NOT consumed. The legacy DWS_AGENTCODE alias was +// hard-removed once the public integration surface landed on +// DINGTALK_DWS_AGENTCODE; hosts must migrate rather than rely on a +// silent fallback. +const agentCodeEnv = "DINGTALK_DWS_AGENTCODE" + +// agentCodePattern is the validation regex for any --agentCode value +// resolved from either the flag or the agent-code env var. It matches +// documented agent-code generation schemes (e.g. md5 digests, uuid-like +// ids, short host-assigned slugs) while rejecting shell metacharacters +// and whitespace that would otherwise flow unescaped into an MCP tool +// argument. +var agentCodePattern = regexp.MustCompile(`^[A-Za-z0-9_-]{1,64}$`) + +// resolveAgentCodeFromEnv returns the fallback agent code from the +// canonical DINGTALK_DWS_AGENTCODE env var. The second return value +// reports the env name that was consumed (for error attribution); it +// is "" when the env is unset or blank. No legacy aliases are honored. +func resolveAgentCodeFromEnv() (string, string) { + primary := strings.TrimSpace(os.Getenv(agentCodeEnv)) + if primary != "" { + return primary, agentCodeEnv + } + return "", "" +} + +// validateAgentCode rejects agent codes that would be ambiguous or unsafe +// once spliced into a shell / MCP argv. Allowed character set is +// [A-Za-z0-9_-], length 1..64 — see agentCodePattern above. +func validateAgentCode(code string) error { + if code == "" { + return fmt.Errorf("--agentCode must not be empty") + } + if !agentCodePattern.MatchString(code) { + return fmt.Errorf( + "invalid agentCode %q: must match %s (A-Z, a-z, 0-9, _, -; 1..64 chars)", + code, agentCodePattern.String()) + } + return nil +} + +// resolveAgentCode implements the canonical two-tier lookup for +// --agentCode: +// +// 1. explicit --agentCode flag value (highest priority; wins over env) +// 2. DINGTALK_DWS_AGENTCODE env var (per-shell primary fallback) +// 3. empty ("") when required=false; typed error when required=true. +// +// Any non-empty resolved value is validated via validateAgentCode, so +// callers never have to re-validate. +func resolveAgentCode(flagVal string, required bool) (string, error) { + code := strings.TrimSpace(flagVal) + envSource := "" + if code == "" { + code, envSource = resolveAgentCodeFromEnv() + } + if code == "" { + if required { + return "", fmt.Errorf( + "flag --agentCode is required (or set env %s)\n hint: dws pat chmod ... --agentCode \n hint: export %s=", + agentCodeEnv, agentCodeEnv) + } + return "", nil + } + if err := validateAgentCode(code); err != nil { + if envSource != "" { + return "", fmt.Errorf("%s env: %w", envSource, err) + } + return "", err + } + return code, nil +} + +const ( + // patGrantToolName is the English-first wire name for the PAT grant tool. + patGrantToolName = "pat.grant" + + // patGrantToolNameLegacyAlias is retained for server builds that still + // expose only the legacy Chinese display name. + patGrantToolNameLegacyAlias = "个人授权" +) + var validGrantTypes = map[string]bool{ "once": true, "session": true, "permanent": true, } -func newChmodCommand(caller edition.ToolCaller) *cobra.Command { +// newChmodCommand builds a fresh `dws pat chmod` cobra.Command wired to +// the supplied ToolCaller. A factory is used (instead of a package-level +// var) so multiple RegisterCommands invocations never share mutable flag / +// RunE state across concurrent tests. +func newChmodCommand(c edition.ToolCaller) *cobra.Command { chmodCmd := &cobra.Command{ Use: "chmod ...", Short: "授予指定权限", @@ -50,9 +172,10 @@ grantType 规则: dws pat chmod chat.message:list --grant-type once --agentCode agt-xxxx dws pat chmod aitable.record:read aitable.record:write --agentCode agt-xxxx --grant-type permanent`, RunE: func(cmd *cobra.Command, args []string) error { - agentCode, _ := cmd.Flags().GetString("agentCode") - if agentCode == "" { - return fmt.Errorf("flag --agentCode is required\n hint: dws pat chmod ... --agentCode ") + flagVal, _ := cmd.Flags().GetString("agentCode") + agentCode, err := resolveAgentCode(flagVal, true) + if err != nil { + return err } scopes := args grantType, _ := cmd.Flags().GetString("grant-type") @@ -62,14 +185,14 @@ grantType 规则: return fmt.Errorf("invalid --grant-type %q, must be one of: once, session, permanent", grantType) } - if grantType == "session" && sessionID == "" && os.Getenv("DWS_SESSION_ID") == "" { + if grantType == "session" && sessionID == "" && resolveSessionIDFromEnv() == "" { return fmt.Errorf("--session-id is required when --grant-type is session\n hint: dws pat chmod --agentCode --grant-type session --session-id ") } - if caller != nil && caller.DryRun() { + if c != nil && c.DryRun() { bold := color.New(color.FgYellow, color.Bold) bold.Println("[DRY-RUN] Preview only, not executed:") - fmt.Printf("%-16s%s\n", "Tool:", "个人授权") + fmt.Printf("%-16s%s\n", "Tool:", patGrantToolName) fmt.Printf("%-16s%s\n", "AgentCode:", agentCode) fmt.Printf("%-16s%v\n", "Scope:", scopes) fmt.Printf("%-16s%s\n", "GrantType:", grantType) @@ -79,24 +202,35 @@ grantType 规则: return nil } - if caller == nil { + if c == nil { return fmt.Errorf("internal error: tool runtime not initialized") } + if sessionID == "" { + sessionID = resolveSessionIDFromEnv() + } toolArgs := map[string]any{ "agentCode": agentCode, - "scope": scopes, + "scopes": scopes, "grantType": grantType, } - if sessionID == "" { - sessionID = os.Getenv("DWS_SESSION_ID") - } if sessionID != "" { toolArgs["sessionId"] = sessionID } + // Legacy server schema accepted singular "scope"; clone the + // canonical argv and rename the key so the two payloads stay + // in lock-step on every other field. + legacyToolArgs := make(map[string]any, len(toolArgs)) + for k, v := range toolArgs { + if k == "scopes" { + legacyToolArgs["scope"] = v + continue + } + legacyToolArgs[k] = v + } ctx := context.Background() - result, err := caller.CallTool(ctx, "pat", "个人授权", toolArgs) + result, err := callPATToolWithLegacyFallback(ctx, c, "pat", patGrantToolName, patGrantToolNameLegacyAlias, toolArgs, legacyToolArgs) if err != nil { return fmt.Errorf("pat chmod failed: %w", err) } @@ -105,14 +239,172 @@ grantType 规则: }, } - chmodCmd.Flags().String("agentCode", "", "Agent 唯一标识（必填）") - _ = chmodCmd.MarkFlagRequired("agentCode") + // --agentCode is required, but we deliberately do NOT call + // MarkFlagRequired here. The agent code may also come from the + // DINGTALK_DWS_AGENTCODE env var; cobra's MarkFlagRequired would + // refuse to run before our resolver has a chance to consume the env. + chmodCmd.Flags().String("agentCode", "", + "Agent 唯一标识（必填；亦可通过 env DINGTALK_DWS_AGENTCODE 注入，flag 优先）") chmodCmd.Flags().String("grant-type", "session", "授权策略: once|session|permanent") chmodCmd.Flags().String("session-id", "", "会话标识（session 模式下必填）") return chmodCmd } +// callPATToolWithLegacyFallback invokes the canonical PAT grant tool first, +// then silently retries the legacy Chinese alias when the server has not +// registered the canonical tool yet. The retry intentionally emits no stderr +// banner because host-owned PAT callers parse stderr as machine JSON. +func callPATToolWithLegacyFallback(ctx context.Context, c edition.ToolCaller, productID, toolName, legacyAlias string, toolArgs, legacyArgs map[string]any) (*edition.ToolResult, error) { + if c == nil { + return nil, fmt.Errorf("internal error: tool runtime not initialized") + } + result, err := c.CallTool(ctx, productID, toolName, toolArgs) + if err == nil { + return result, nil + } + if legacyAlias == "" { + return nil, err + } + if !isToolNotRegisteredError(err) && !isLegacyGrantSchemaMismatchError(err, toolArgs, legacyArgs) { + return nil, err + } + return c.CallTool(ctx, productID, legacyAlias, legacyArgs) +} + +func isEmptyToolResult(result *edition.ToolResult) bool { + if result == nil || len(result.Content) == 0 { + return true + } + for _, block := range result.Content { + if block.Type == "text" && strings.TrimSpace(block.Text) != "" { + return false + } + } + return true +} + +// isToolNotRegisteredError reports whether err looks like a server-side +// tool-not-registered / tool-not-found classification. We match on a few +// conservative substrings rather than a structured error type because the +// upstream runner surfaces the server message as plain text. +func isToolNotRegisteredError(err error) bool { + if err == nil { + return false + } + msg := normalizedPATErrorText(err) + needles := []string{ + "tool_not_found", + "mcp_tool_not_found", + "tool not found", + "tool not registered", + "tool not exist", + "tool does not exist", + "unknown tool", + "no such tool", + "未找到指定工具", + "未找到工具", + "工具不存在", + } + for _, needle := range needles { + if strings.Contains(msg, needle) { + return true + } + } + return false +} + +func isLegacyGrantSchemaMismatchError(err error, toolArgs, legacyArgs map[string]any) bool { + if err == nil || !hasScopeKeyShapeMismatch(toolArgs, legacyArgs) { + return false + } + if apperrors.IsPATError(err) { + return false + } + msg := normalizedPATErrorText(err) + if containsAny(msg, + "pat_no_permission", + "pat_low_risk_no_permission", + "pat_medium_risk_no_permission", + "pat_high_risk_no_permission", + "pat_scope_auth_required", + "agent_code_not_exists", + "requiredscopes", + "missingscope", + "missing_scope", + "insufficient_scope", + ) { + return false + } + if !containsAny(msg, "scope", "scopes") { + return false + } + if !containsAny(msg, + "param_error", + "参数错误", + "parameter", + "validation", + "required", + "missing", + "unknown", + "unexpected", + "invalid", + "unmarshal", + ) { + return false + } + if containsAny(msg, + "permission denied", + "no permission", + "forbidden", + "unauthorized", + "auth required", + "无权限", + "未授权", + "pat_medium_risk_no_permission", + ) { + return false + } + return true +} + +func hasScopeKeyShapeMismatch(toolArgs, legacyArgs map[string]any) bool { + if toolArgs == nil || legacyArgs == nil { + return false + } + _, hasCanonicalPlural := toolArgs["scopes"] + _, hasCanonicalSingular := toolArgs["scope"] + _, hasLegacyPlural := legacyArgs["scopes"] + _, hasLegacySingular := legacyArgs["scope"] + return hasCanonicalPlural && !hasCanonicalSingular && hasLegacySingular && !hasLegacyPlural +} + +func normalizedPATErrorText(err error) string { + if err == nil { + return "" + } + parts := []string{strings.ToLower(err.Error())} + var typed *apperrors.Error + if stderrors.As(err, &typed) && typed != nil { + parts = append(parts, + strings.ToLower(typed.Reason), + strings.ToLower(typed.ServerDiag.ServerErrorCode), + strings.ToLower(typed.ServerDiag.TechnicalDetail), + strings.ToLower(typed.Hint), + ) + } + return strings.Join(parts, " ") +} + +func containsAny(msg string, needles ...string) bool { + for _, needle := range needles { + if strings.Contains(msg, needle) { + return true + } + } + return false +} + // handleToolResult processes a ToolResult and writes output to stdout. func handleToolResult(result *edition.ToolResult) error { if result == nil { @@ -128,10 +420,6 @@ func handleToolResult(result *edition.ToolResult) error { fmt.Println(c.Text) return nil } - data, err := json.MarshalIndent(result, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal result: %w", err) - } - fmt.Println(string(data)) - return nil + data, _ := json.Marshal(result) + return fmt.Errorf("empty PAT authorization result: %s", string(data)) } diff --git a/internal/pat/chmod_test.go b/internal/pat/chmod_test.go new file mode 100644 index 00000000..fc60294b --- /dev/null +++ b/internal/pat/chmod_test.go @@ -0,0 +1,613 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pat + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +// fakeToolCaller captures the toolArgs passed to CallTool so tests can +// assert how the two-tier --agentCode / DINGTALK_DWS_AGENTCODE / error +// resolver feeds into the outgoing MCP argv. +type fakeToolCaller struct { + mu sync.Mutex + dryRun bool + gotTool string + gotArgs map[string]any + callN int + resultOK bool +} + +func (f *fakeToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.callN++ + f.gotTool = toolName + // defensive copy — RunE / runApply may mutate the map after return + f.gotArgs = make(map[string]any, len(args)) + for k, v := range args { + f.gotArgs[k] = v + } + // Empty success payload keeps handleToolResult / emitApplyResult happy + // without triggering PAT classification in errors.ClassifyMCPResponseText. + if f.resultOK { + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{"success":true,"data":{}}`}}}, nil + } + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{"success":true,"data":{"authRequestId":"req-ok"}}`}}}, nil +} + +func (f *fakeToolCaller) Format() string { return "json" } +func (f *fakeToolCaller) DryRun() bool { return f.dryRun } + +type recordedToolCall struct { + tool string + args map[string]any +} + +type fallbackToolCaller struct { + calls []recordedToolCall +} + +func (f *fallbackToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + copied := make(map[string]any, len(args)) + for k, v := range args { + copied[k] = v + } + f.calls = append(f.calls, recordedToolCall{tool: toolName, args: copied}) + if len(f.calls) == 1 { + return &edition.ToolResult{}, nil + } + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{"success":true,"data":{"authRequestId":"req-ok"}}`}}}, nil +} + +func (f *fallbackToolCaller) Format() string { return "json" } +func (f *fallbackToolCaller) DryRun() bool { return false } + +type fallbackErrorToolCaller struct { + calls []recordedToolCall +} + +func (f *fallbackErrorToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + copied := make(map[string]any, len(args)) + for k, v := range args { + copied[k] = v + } + f.calls = append(f.calls, recordedToolCall{tool: toolName, args: copied}) + if len(f.calls) == 1 { + return nil, errors.New("pat chmod failed: business error: PARAM_ERROR - 未找到指定工具") + } + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{"success":true,"data":{"authRequestId":"req-ok"}}`}}}, nil +} + +func (f *fallbackErrorToolCaller) Format() string { return "json" } +func (f *fallbackErrorToolCaller) DryRun() bool { return false } + +type fallbackSchemaMismatchToolCaller struct { + calls []recordedToolCall +} + +func (f *fallbackSchemaMismatchToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + copied := make(map[string]any, len(args)) + for k, v := range args { + copied[k] = v + } + f.calls = append(f.calls, recordedToolCall{tool: toolName, args: copied}) + if len(f.calls) == 1 { + return nil, apperrors.NewAPI("business error: success=false", + apperrors.WithReason("business_error"), + apperrors.WithServerDiag(apperrors.ServerDiagnostics{ + ServerErrorCode: "PARAM_ERROR", + TechnicalDetail: `input schema validation failed: unknown field "scopes"; missing required field "scope"`, + }), + ) + } + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{"success":true,"data":{"authRequestId":"req-ok"}}`}}}, nil +} + +func (f *fallbackSchemaMismatchToolCaller) Format() string { return "json" } +func (f *fallbackSchemaMismatchToolCaller) DryRun() bool { return false } + +type fallbackPermissionDeniedToolCaller struct { + calls []recordedToolCall +} + +func (f *fallbackPermissionDeniedToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + copied := make(map[string]any, len(args)) + for k, v := range args { + copied[k] = v + } + f.calls = append(f.calls, recordedToolCall{tool: toolName, args: copied}) + return nil, apperrors.NewAPI("business error: success=false", + apperrors.WithReason("business_error"), + apperrors.WithServerDiag(apperrors.ServerDiagnostics{ + ServerErrorCode: "PAT_MEDIUM_RISK_NO_PERMISSION", + TechnicalDetail: "permission denied for scope chat.message:send", + }), + ) +} + +func (f *fallbackPermissionDeniedToolCaller) Format() string { return "json" } +func (f *fallbackPermissionDeniedToolCaller) DryRun() bool { return false } + +type fallbackPATErrorToolCaller struct { + calls []recordedToolCall +} + +func (f *fallbackPATErrorToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + copied := make(map[string]any, len(args)) + for k, v := range args { + copied[k] = v + } + f.calls = append(f.calls, recordedToolCall{tool: toolName, args: copied}) + return nil, &apperrors.PATError{RawJSON: `{"success":false,"code":"PAT_SCOPE_AUTH_REQUIRED","data":{"missingScope":"mail:send"}}`} +} + +func (f *fallbackPATErrorToolCaller) Format() string { return "json" } +func (f *fallbackPATErrorToolCaller) DryRun() bool { return false } + +type fallbackPATContractErrorToolCaller struct { + calls []recordedToolCall +} + +func (f *fallbackPATContractErrorToolCaller) CallTool(_ context.Context, _ string, toolName string, args map[string]any) (*edition.ToolResult, error) { + copied := make(map[string]any, len(args)) + for k, v := range args { + copied[k] = v + } + f.calls = append(f.calls, recordedToolCall{tool: toolName, args: copied}) + return nil, apperrors.NewAPI("business error: success=false", + apperrors.WithReason("business_error"), + apperrors.WithServerDiag(apperrors.ServerDiagnostics{ + ServerErrorCode: "PAT_SCOPE_AUTH_REQUIRED", + TechnicalDetail: `missingScope mail:send`, + }), + ) +} + +func (f *fallbackPATContractErrorToolCaller) Format() string { return "json" } +func (f *fallbackPATContractErrorToolCaller) DryRun() bool { return false } + +func stringSliceArgEqual(got any, want []string) bool { + gotSlice, ok := got.([]string) + if !ok || len(gotSlice) != len(want) { + return false + } + for i := range want { + if gotSlice[i] != want[i] { + return false + } + } + return true +} + +// buildChmod returns a freshly constructed chmod cobra.Command wired to +// fake. Using the factory (instead of a package-level var) keeps every +// subtest hermetic and matches the upstream shared-state fix in PR #129. +func buildChmod(t *testing.T, fake *fakeToolCaller) *cobra.Command { + t.Helper() + return newChmodCommand(fake) +} + +// --------------------------------------------------------------------------- +// T1 · Agent-code env fallback tests +// --------------------------------------------------------------------------- + +// TestChmod_agentCode_env_fallback verifies that when --agentCode is +// omitted but DINGTALK_DWS_AGENTCODE is exported, the resolver picks +// the env value up and forwards it verbatim in the MCP argv. +func TestChmod_agentCode_env_fallback(t *testing.T) { + t.Setenv(agentCodeEnv, "qoderwork") + + fake := &fakeToolCaller{resultOK: true} + cmd := buildChmod(t, fake) + + // grant-type=once → no session-id needed; keeps the test hermetic. + _ = cmd.Flags().Set("grant-type", "once") + if err := cmd.RunE(cmd, []string{"aitable.record:read"}); err != nil { + t.Fatalf("chmod RunE error = %v (must not report flag missing)", err) + } + + if got := fake.gotArgs["agentCode"]; got != "qoderwork" { + t.Fatalf("agentCode in argv = %v, want %q (env fallback)", got, "qoderwork") + } + if got := fake.gotArgs["scopes"]; !stringSliceArgEqual(got, []string{"aitable.record:read"}) { + t.Fatalf("scopes in argv = %#v, want %#v", got, []string{"aitable.record:read"}) + } + if _, ok := fake.gotArgs["scope"]; ok { + t.Fatalf("unexpected legacy singular scope arg in argv: %#v", fake.gotArgs) + } +} + +func TestCallPATToolWithLegacyFallback_emptyCanonicalResultDoesNotRetryLegacyAlias(t *testing.T) { + fake := &fallbackToolCaller{} + canonicalArgs := map[string]any{ + "agentCode": "qoderwork", + "scopes": []string{"aitable.record:read"}, + "grantType": "permanent", + } + legacyArgs := map[string]any{ + "agentCode": "qoderwork", + "scope": []string{"aitable.record:read"}, + "grantType": "permanent", + } + + result, err := callPATToolWithLegacyFallback(context.Background(), fake, "pat", patGrantToolName, patGrantToolNameLegacyAlias, canonicalArgs, legacyArgs) + if err != nil { + t.Fatalf("callPATToolWithLegacyFallback error = %v", err) + } + if !isEmptyToolResult(result) { + t.Fatalf("expected original empty canonical result, got %#v", result) + } + if len(fake.calls) != 1 { + t.Fatalf("CallTool call count = %d, want 1", len(fake.calls)) + } + if fake.calls[0].tool != patGrantToolName { + t.Fatalf("first tool = %q, want %q", fake.calls[0].tool, patGrantToolName) + } + if _, ok := fake.calls[0].args["scopes"]; !ok { + t.Fatalf("canonical args missing scopes: %#v", fake.calls[0].args) + } + if _, ok := fake.calls[0].args["scope"]; ok { + t.Fatalf("canonical args should not use legacy scope: %#v", fake.calls[0].args) + } +} + +func TestChmod_emptyCanonicalResultReturnsError(t *testing.T) { + fake := &fallbackToolCaller{} + cmd := newChmodCommand(fake) + _ = cmd.Flags().Set("agentCode", "qoderwork") + _ = cmd.Flags().Set("grant-type", "permanent") + + err := cmd.RunE(cmd, []string{"aitable.record:read"}) + if err == nil { + t.Fatal("chmod RunE error = nil, want empty PAT authorization result") + } + if !strings.Contains(err.Error(), "empty PAT authorization result") { + t.Fatalf("chmod RunE error = %q, want empty PAT authorization result", err.Error()) + } + if len(fake.calls) != 1 { + t.Fatalf("CallTool call count = %d, want 1", len(fake.calls)) + } + if fake.calls[0].tool != patGrantToolName { + t.Fatalf("first tool = %q, want %q", fake.calls[0].tool, patGrantToolName) + } +} + +func TestCallPATToolWithLegacyFallback_toolNotFoundRetriesLegacyAlias(t *testing.T) { + fake := &fallbackErrorToolCaller{} + canonicalArgs := map[string]any{ + "agentCode": "qoderwork", + "scopes": []string{"aitable.record:read"}, + "grantType": "permanent", + } + legacyArgs := map[string]any{ + "agentCode": "qoderwork", + "scope": []string{"aitable.record:read"}, + "grantType": "permanent", + } + + result, err := callPATToolWithLegacyFallback(context.Background(), fake, "pat", patGrantToolName, patGrantToolNameLegacyAlias, canonicalArgs, legacyArgs) + if err != nil { + t.Fatalf("callPATToolWithLegacyFallback error = %v", err) + } + if isEmptyToolResult(result) { + t.Fatalf("fallback result is empty: %#v", result) + } + if len(fake.calls) != 2 { + t.Fatalf("CallTool call count = %d, want 2", len(fake.calls)) + } + if fake.calls[0].tool != patGrantToolName { + t.Fatalf("first tool = %q, want %q", fake.calls[0].tool, patGrantToolName) + } + if fake.calls[1].tool != patGrantToolNameLegacyAlias { + t.Fatalf("fallback tool = %q, want %q", fake.calls[1].tool, patGrantToolNameLegacyAlias) + } + if _, ok := fake.calls[1].args["scope"]; !ok { + t.Fatalf("legacy args missing scope: %#v", fake.calls[1].args) + } + if _, ok := fake.calls[1].args["scopes"]; ok { + t.Fatalf("legacy args should not use canonical scopes: %#v", fake.calls[1].args) + } +} + +func TestCallPATToolWithLegacyFallback_schemaMismatchRetriesLegacyAlias(t *testing.T) { + fake := &fallbackSchemaMismatchToolCaller{} + canonicalArgs := map[string]any{ + "agentCode": "qoderwork", + "scopes": []string{"aitable.record:read"}, + "grantType": "permanent", + } + legacyArgs := map[string]any{ + "agentCode": "qoderwork", + "scope": []string{"aitable.record:read"}, + "grantType": "permanent", + } + + result, err := callPATToolWithLegacyFallback(context.Background(), fake, "pat", patGrantToolName, patGrantToolNameLegacyAlias, canonicalArgs, legacyArgs) + if err != nil { + t.Fatalf("callPATToolWithLegacyFallback error = %v", err) + } + if isEmptyToolResult(result) { + t.Fatalf("fallback result is empty: %#v", result) + } + if len(fake.calls) != 2 { + t.Fatalf("CallTool call count = %d, want 2", len(fake.calls)) + } + if fake.calls[0].tool != patGrantToolName { + t.Fatalf("first tool = %q, want %q", fake.calls[0].tool, patGrantToolName) + } + if fake.calls[1].tool != patGrantToolNameLegacyAlias { + t.Fatalf("fallback tool = %q, want %q", fake.calls[1].tool, patGrantToolNameLegacyAlias) + } + if _, ok := fake.calls[1].args["scope"]; !ok { + t.Fatalf("legacy args missing scope: %#v", fake.calls[1].args) + } +} + +func TestCallPATToolWithLegacyFallback_permissionDeniedDoesNotRetryLegacyAlias(t *testing.T) { + fake := &fallbackPermissionDeniedToolCaller{} + canonicalArgs := map[string]any{ + "agentCode": "qoderwork", + "scopes": []string{"chat.message:send"}, + "grantType": "once", + } + legacyArgs := map[string]any{ + "agentCode": "qoderwork", + "scope": []string{"chat.message:send"}, + "grantType": "once", + } + + _, err := callPATToolWithLegacyFallback(context.Background(), fake, "pat", patGrantToolName, patGrantToolNameLegacyAlias, canonicalArgs, legacyArgs) + if err == nil { + t.Fatal("callPATToolWithLegacyFallback error = nil, want original permission denial") + } + var typed *apperrors.Error + if !errors.As(err, &typed) { + t.Fatalf("error type = %T, want *errors.Error", err) + } + if typed.ServerDiag.ServerErrorCode != "PAT_MEDIUM_RISK_NO_PERMISSION" { + t.Fatalf("ServerErrorCode = %q, want PAT_MEDIUM_RISK_NO_PERMISSION", typed.ServerDiag.ServerErrorCode) + } + if len(fake.calls) != 1 { + t.Fatalf("CallTool call count = %d, want 1", len(fake.calls)) + } +} + +func TestCallPATToolWithLegacyFallback_patErrorDoesNotRetryLegacyAlias(t *testing.T) { + fake := &fallbackPATErrorToolCaller{} + canonicalArgs := map[string]any{ + "agentCode": "qoderwork", + "scopes": []string{"mail:send"}, + "grantType": "once", + } + legacyArgs := map[string]any{ + "agentCode": "qoderwork", + "scope": []string{"mail:send"}, + "grantType": "once", + } + + _, err := callPATToolWithLegacyFallback(context.Background(), fake, "pat", patGrantToolName, patGrantToolNameLegacyAlias, canonicalArgs, legacyArgs) + if err == nil { + t.Fatal("callPATToolWithLegacyFallback error = nil, want PATError") + } + if !apperrors.IsPATError(err) { + t.Fatalf("expected PATError, got %T", err) + } + if len(fake.calls) != 1 { + t.Fatalf("CallTool call count = %d, want 1", len(fake.calls)) + } +} + +func TestCallPATToolWithLegacyFallback_patContractErrorDoesNotRetryLegacyAlias(t *testing.T) { + fake := &fallbackPATContractErrorToolCaller{} + canonicalArgs := map[string]any{ + "agentCode": "qoderwork", + "scopes": []string{"mail:send"}, + "grantType": "once", + } + legacyArgs := map[string]any{ + "agentCode": "qoderwork", + "scope": []string{"mail:send"}, + "grantType": "once", + } + + _, err := callPATToolWithLegacyFallback(context.Background(), fake, "pat", patGrantToolName, patGrantToolNameLegacyAlias, canonicalArgs, legacyArgs) + if err == nil { + t.Fatal("callPATToolWithLegacyFallback error = nil, want original PAT contract error") + } + var typed *apperrors.Error + if !errors.As(err, &typed) { + t.Fatalf("error type = %T, want *errors.Error", err) + } + if typed.ServerDiag.ServerErrorCode != "PAT_SCOPE_AUTH_REQUIRED" { + t.Fatalf("ServerErrorCode = %q, want PAT_SCOPE_AUTH_REQUIRED", typed.ServerDiag.ServerErrorCode) + } + if len(fake.calls) != 1 { + t.Fatalf("CallTool call count = %d, want 1", len(fake.calls)) + } +} + +func TestIsToolNotRegisteredError_ChineseGatewayMessage(t *testing.T) { + err := errors.New("pat chmod failed: business error: PARAM_ERROR - 未找到指定工具") + if !isToolNotRegisteredError(err) { + t.Fatalf("isToolNotRegisteredError(%q) = false, want true", err.Error()) + } +} + +func TestIsToolNotRegisteredError_ChineseGatewayDiagnostics(t *testing.T) { + err := apperrors.NewAPI("business error: success=false", + apperrors.WithReason("business_error"), + apperrors.WithServerDiag(apperrors.ServerDiagnostics{ + ServerErrorCode: "PARAM_ERROR", + TechnicalDetail: "Tool metadata API error: PARAM_ERROR - 未找到指定工具", + }), + ) + if !isToolNotRegisteredError(err) { + t.Fatalf("isToolNotRegisteredError(%q) = false, want true", err.Error()) + } +} + +func TestHandleToolResult_emptyResultReturnsError(t *testing.T) { + err := handleToolResult(&edition.ToolResult{}) + if err == nil { + t.Fatal("handleToolResult error = nil, want empty PAT authorization result error") + } + if !strings.Contains(err.Error(), "empty PAT authorization result") { + t.Fatalf("handleToolResult error = %q, want empty PAT authorization result", err.Error()) + } +} + +// TestChmod_agentCode_env_invalid verifies that a malformed +// DINGTALK_DWS_AGENTCODE value (whitespace, shell metacharacters) is +// rejected by the regex gate in validateAgentCode before any MCP call +// is attempted. +func TestChmod_agentCode_env_invalid(t *testing.T) { + t.Setenv(agentCodeEnv, "bad value with space!") + + fake := &fakeToolCaller{resultOK: true} + cmd := buildChmod(t, fake) + _ = cmd.Flags().Set("grant-type", "once") + + err := cmd.RunE(cmd, []string{"aitable.record:read"}) + if err == nil { + t.Fatalf("expected validateAgentCode error, got nil") + } + if !strings.Contains(err.Error(), "invalid agentCode") { + t.Fatalf("error = %q, want to mention 'invalid agentCode'", err.Error()) + } + if !strings.Contains(err.Error(), agentCodeEnv) { + t.Fatalf("error = %q, want to attribute to %s env", err.Error(), agentCodeEnv) + } + if fake.callN != 0 { + t.Fatalf("CallTool was invoked %d times; validator must short-circuit before MCP", fake.callN) + } +} + +// TestChmod_agentCode_flag_wins_over_env verifies the Priority-1 contract +// of resolveAgentCode: when both the flag and the env are set, the flag +// wins and env is silently ignored (no warning needed because the flag is +// the explicit, scripted intent). +func TestChmod_agentCode_flag_wins_over_env(t *testing.T) { + t.Setenv(agentCodeEnv, "envval") + + fake := &fakeToolCaller{resultOK: true} + cmd := buildChmod(t, fake) + + _ = cmd.Flags().Set("grant-type", "once") + _ = cmd.Flags().Set("agentCode", "flagval") + + if err := cmd.RunE(cmd, []string{"aitable.record:read"}); err != nil { + t.Fatalf("chmod RunE error = %v", err) + } + if got := fake.gotArgs["agentCode"]; got != "flagval" { + t.Fatalf("agentCode in argv = %v, want %q (flag must win over env)", got, "flagval") + } +} + +// TestChmod_agentCode_legacy_env_not_recognized is a reverse-guard: after +// the SSOT hard-removal of the DWS_AGENTCODE alias, exporting only the +// legacy env MUST NOT satisfy the --agentCode requirement. The command +// is expected to fail with an error that explicitly names the canonical +// DINGTALK_DWS_AGENTCODE env, and MUST NOT mention DWS_AGENTCODE as a +// usable fallback. No MCP call is permitted. +func TestChmod_agentCode_legacy_env_not_recognized(t *testing.T) { + t.Setenv(agentCodeEnv, "") + t.Setenv("DWS_AGENTCODE", "legacyval") + + fake := &fakeToolCaller{resultOK: true} + cmd := buildChmod(t, fake) + _ = cmd.Flags().Set("grant-type", "once") + + err := cmd.RunE(cmd, []string{"aitable.record:read"}) + if err == nil { + t.Fatalf("expected hard error when only legacy DWS_AGENTCODE is set, got nil") + } + if !strings.Contains(err.Error(), "DINGTALK_DWS_AGENTCODE") { + t.Fatalf("error = %q, want to name canonical DINGTALK_DWS_AGENTCODE env", err.Error()) + } + // Defensive: the canonical env naturally contains the substring + // "DWS_AGENTCODE" as part of "DINGTALK_DWS_AGENTCODE"; the above + // assertion plus the absence check below precisely guard against + // advertising the legacy alias as usable. + hint := strings.ReplaceAll(err.Error(), "DINGTALK_DWS_AGENTCODE", "") + if strings.Contains(hint, "DWS_AGENTCODE") { + t.Fatalf("error = %q must not advertise DWS_AGENTCODE as usable", err.Error()) + } + if fake.callN != 0 { + t.Fatalf("CallTool was invoked %d times; legacy env must not satisfy --agentCode", fake.callN) + } +} + +// --------------------------------------------------------------------------- +// validateAgentCode / resolveAgentCodeFromEnv unit tests +// --------------------------------------------------------------------------- + +func TestValidateAgentCode(t *testing.T) { + t.Parallel() + cases := []struct { + in string + wantErr bool + }{ + {"qoderwork", false}, + {"agt-abc123", false}, + {"Agt_Xyz-09", false}, + {"abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789", false}, // 64 chars + {"", true}, + {"bad value", true}, + {"bad!chars", true}, + {"中文不行", true}, + {"abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789X", true}, // 65 + } + for _, tc := range cases { + err := validateAgentCode(tc.in) + if (err != nil) != tc.wantErr { + t.Errorf("validateAgentCode(%q) err=%v, wantErr=%v", tc.in, err, tc.wantErr) + } + } +} + +func TestResolveAgentCodeFromEnv(t *testing.T) { + // Not parallel: mutates process env. + + // DINGTALK_DWS_AGENTCODE is honoured and trimmed. + t.Setenv(agentCodeEnv, " qoderwork ") + if code, src := resolveAgentCodeFromEnv(); code != "qoderwork" || src != agentCodeEnv { + t.Errorf("resolveAgentCodeFromEnv() = (%q, %q), want (%q, %q)", + code, src, "qoderwork", agentCodeEnv) + } + + // Empty primary → ("", ""). + t.Setenv(agentCodeEnv, "") + if code, src := resolveAgentCodeFromEnv(); code != "" || src != "" { + t.Errorf("resolveAgentCodeFromEnv() = (%q, %q), want empty", code, src) + } + + // Reverse-guard: legacy DWS_AGENTCODE MUST NOT be picked up when the + // canonical env is unset — it was hard-removed as a legacy alias. + t.Setenv(agentCodeEnv, "") + t.Setenv("DWS_AGENTCODE", "legacy") + if code, src := resolveAgentCodeFromEnv(); code != "" || src != "" { + t.Errorf("resolveAgentCodeFromEnv() = (%q, %q), want empty — legacy DWS_AGENTCODE must be ignored", + code, src) + } +} diff --git a/internal/pat/pat.go b/internal/pat/pat.go index 1907dc01..0d918c95 100644 --- a/internal/pat/pat.go +++ b/internal/pat/pat.go @@ -30,10 +30,37 @@ func RegisterCommands(root *cobra.Command, c edition.ToolCaller) { Long: `管理行为授权（PAT）。 命令结构: - dws pat chmod ... 授予指定权限`, + dws pat chmod ... 授予指定权限 + dws pat browser-policy 配置 PAT 浏览器打开策略 + +能力说明： + --format 只控制 PAT 撞墙时的输出形态；当 --format json 时， + CLI 只返回结构化 JSON，不混入非结构化文本。 + 浏览器是否打开由本地 PAT 策略单独决定，与 json / non-json 独立。 + 生效时会优先按 DINGTALK_DWS_AGENTCODE 读取 agent 策略，再回退到默认策略。 + 写入 agent 策略需显式传 --agentCode；不传则写入全局默认策略。 + +Host-owned PAT 开关： + 当且仅当环境变量 DINGTALK_DWS_AGENTCODE 非空时，CLI 命中 PAT + 固定以 stderr JSON + exit=4 的 host-owned 形式返回， + 由宿主处理全部 UI / 交互 / 回调节奏 / 重试逻辑， + CLI 侧不再拉起任何本地浏览器 / 轮询。 + +服务端路由标签 claw-type（开源构建硬编码）： + 开源构建在所有出站 MCP 请求上恒定注入 claw-type: openClaw， + 与 DINGTALK_AGENT / 宿主环境解耦，与历史 main 行为一致。 + hostControl.clawType 也会回填该值，便于宿主侧审计/路由。 + +DINGTALK_AGENT（可选，仅供 x-dingtalk-agent 使用）： + 如设置，将原样注入 HTTP 请求头 x-dingtalk-agent， + 便于上游按业务 Agent 名称区分流量。 + 它不参与 claw-type 派生，也不参与 host-owned PAT 判定。 + +DWS_CHANNEL 只用于上游 channelCode。`, RunE: cmdutil.GroupRunE, } patCmd.AddCommand(newChmodCommand(c)) + patCmd.AddCommand(newBrowserPolicyCommand()) root.AddCommand(patCmd) } diff --git a/pkg/edition/default.go b/pkg/edition/default.go index 5bdddae6..2572b5b1 100644 --- a/pkg/edition/default.go +++ b/pkg/edition/default.go @@ -13,9 +13,20 @@ package edition +// DefaultOSSClawType is the wire value for request header claw-type in +// the open-source build. It is intentionally hard-wired — the open-source +// CLI does NOT derive claw-type from DINGTALK_AGENT or any other caller +// input, so third-party hosts get a predictable header regardless of +// their environment. +const DefaultOSSClawType = "openClaw" + // defaultHooks returns the open-source edition defaults. -// All function hooks are nil, which the internal code interprets as -// "use standard open-source behaviour". +// +// MergeHeaders is the only hook that ships with behaviour: it pins the +// `claw-type` request header to DefaultOSSClawType so every open-source +// MCP request carries the same stable routing tag. All other fields are +// nil — the internal code interprets nil as "use standard open-source +// behaviour". func defaultHooks() *Hooks { return &Hooks{ Name: "open", @@ -23,7 +34,7 @@ func defaultHooks() *Hooks { if base == nil { base = make(map[string]string) } - base["claw-type"] = "openClaw" + base["claw-type"] = DefaultOSSClawType return base }, } diff --git a/test/unit/open_source_policy_test.go b/test/unit/open_source_policy_test.go index e3f194d9..2084f59b 100644 --- a/test/unit/open_source_policy_test.go +++ b/test/unit/open_source_policy_test.go @@ -10,6 +10,14 @@ import ( "testing" ) +// TestOpenSourceTreeOmitsEmbeddedHostMarkers scans source files for proprietary +// markers that must not leak into the public (open-source) tree. +// +// The scanner covers public docs as well as source code because OSS leakage is +// often introduced through documentation first. Only a small set of explicitly +// documented compatibility literals (see NOTE below) are allowed to remain in +// tree; everything else should trip the guard regardless of whether it appears +// in Go, shell, YAML, templates, or markdown. func TestOpenSourceTreeOmitsEmbeddedHostMarkers(t *testing.T) { _, filename, _, ok := runtime.Caller(0) if !ok { @@ -17,6 +25,14 @@ func TestOpenSourceTreeOmitsEmbeddedHostMarkers(t *testing.T) { } root := filepath.Clean(filepath.Join(filepath.Dir(filename), "..", "..")) + // NOTE: REWIND_SESSION_ID / REWIND_REQUEST_ID / REWIND_MESSAGE_ID are + // intentionally NOT on this list. They are accepted by the CLI as + // optional backward-compatibility aliases for the primary DWS_* trace + // env names. Because they are a documented compatibility surface + // rather than an internal coupling to a specific host implementation, + // referring to these literals from source code and docs is allowed. + // Product names like "RewindDesktop" and other host-implementation + // specific symbols remain forbidden. forbidden := []string{ "DWS_" + "BUILD_MODE", "com.dingtalk.scenario." + "wukong", @@ -27,12 +43,6 @@ func TestOpenSourceTreeOmitsEmbeddedHostMarkers(t *testing.T) { "EnablePrivate" + "UtilityCommands", "UseExecutable" + "ConfigDir", "DeleteExeRelative" + "TokenOnAuthErr", - "WriteToken" + "Marker", - "Token" + "Marker", - "tokenJSON" + "File", - "REWIND_" + "REQUEST_ID", - "REWIND_" + "SESSION_ID", - "REWIND_" + "MESSAGE_ID", "MergeWukong" + "MCPHeaders", "buildMode ==" + " \"real\"", "wukong/" + "discovery", diff --git a/test/unit/pat_host_owned_signal_test.go b/test/unit/pat_host_owned_signal_test.go new file mode 100644 index 00000000..77de05d3 --- /dev/null +++ b/test/unit/pat_host_owned_signal_test.go @@ -0,0 +1,102 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package unit_test + +import ( + "testing" + + authpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/auth" +) + +// TestHostOwnsPATFlow_OnlySignal is the wire-level guard for the +// "custom authorization card" contract: the CLI switches to host-owned +// PAT mode iff the host injects DINGTALK_DWS_AGENTCODE. DINGTALK_AGENT / +// claw-type is purely a server-side routing tag and must NOT influence +// the decision, in either direction. +// +// Regression guard: several earlier drafts conflated the two signals, +// causing third-party Agent hosts that only set DINGTALK_DWS_AGENTCODE +// to still see a browser popup (CLI-owned fallback). This table locks +// in the decoupling. +func TestHostOwnsPATFlow_OnlySignal(t *testing.T) { + cases := []struct { + name string + agentCode string + agentEnv string + want bool + }{ + { + name: "no signal → CLI-owned", + agentCode: "", + agentEnv: "", + want: false, + }, + { + name: "agent code only → host-owned", + agentCode: "agt-cursor", + agentEnv: "", + want: true, + }, + { + name: "agent code + DINGTALK_AGENT=default → host-owned", + agentCode: "agt-cursor", + agentEnv: "default", + want: true, + }, + { + name: "agent code + DINGTALK_AGENT=business → host-owned", + agentCode: "agt-cursor", + agentEnv: "sales-copilot", + want: true, + }, + { + name: "DINGTALK_AGENT=business, no agent code → CLI-owned", + agentCode: "", + agentEnv: "sales-copilot", + want: false, + }, + { + name: "DINGTALK_AGENT=default, no agent code → CLI-owned", + agentCode: "", + agentEnv: "default", + want: false, + }, + { + name: "whitespace-only agent code → CLI-owned", + agentCode: " ", + agentEnv: "sales-copilot", + want: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Setenv(authpkg.AgentCodeEnv, tc.agentCode) + // DINGTALK_AGENT is set purely to demonstrate that it does NOT + // influence the host-owned decision. The literal env name is + // used here because the auth package no longer exports a + // DingTalkAgentEnv constant (it is not part of the PAT + // decision surface). + t.Setenv("DINGTALK_AGENT", tc.agentEnv) + + if got := authpkg.HostOwnsPATFlow(); got != tc.want { + t.Fatalf( + "HostOwnsPATFlow() = %v, want %v (agentCode=%q, DINGTALK_AGENT=%q)", + got, tc.want, tc.agentCode, tc.agentEnv, + ) + } + }) + } +} diff --git a/test/unit/pat_stderr_contract_test.go b/test/unit/pat_stderr_contract_test.go new file mode 100644 index 00000000..7e6f3df9 --- /dev/null +++ b/test/unit/pat_stderr_contract_test.go @@ -0,0 +1,268 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package unit_test + +import ( + "encoding/json" + "strings" + "testing" + + errpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" +) + +// D5 Smoke A — PAT stderr JSON is a single-line, json.Unmarshal-able +// payload for every code in the frozen enum. +// +// This test is the CI-level guard for the wire invariant ("stderr JSON +// MUST be single-line and directly unmarshal-able"). It sits in the +// public `test/unit` tree — deliberately separate from +// internal/errors/pat_test.go — so the contract stays defended even +// after internal refactors that might move the JSON assembly code to +// another package or collapse helpers into it. +// +// Wire path exercised: ClassifyPatAuthCheck covers BOTH error families +// the SSOT freezes — patNoPermissionCodes (PAT_NO_PERMISSION and the +// three risk-tier variants) and patAuthRequiredCodes +// (PAT_SCOPE_AUTH_REQUIRED, AGENT_CODE_NOT_EXISTS). Using this single +// entry point keeps the test format-agnostic to future code-churn in +// ClassifyMCPResponseText's branching while still covering the full +// SSOT code enum. +// +// What we assert for every case: +// 1. Non-nil *PATError is returned with ExitCode == 4. +// 2. RawJSON has NO '\n' characters (single-line invariant). +// 3. RawJSON is directly json.Unmarshal-able into a map. +// 4. Parsed body has success=false and code matching the fixture. +// 5. Required downstream data shape is present (requiredScopes for +// PAT_*_NO_PERMISSION; missingScope for PAT_SCOPE_AUTH_REQUIRED). +func TestPATStderrJSON_SingleLineUnmarshalable(t *testing.T) { + t.Parallel() + + type assertFn func(t *testing.T, parsed map[string]any) + + requiredScopesPresent := func(t *testing.T, parsed map[string]any) { + t.Helper() + data, ok := parsed["data"].(map[string]any) + if !ok { + t.Fatalf("parsed.data missing or wrong type: %v", parsed["data"]) + } + scopes, ok := data["requiredScopes"].([]any) + if !ok || len(scopes) == 0 { + t.Fatalf("parsed.data.requiredScopes missing or empty: %v", data["requiredScopes"]) + } + for i, s := range scopes { + if _, ok := s.(string); !ok { + t.Fatalf("parsed.data.requiredScopes[%d] is not a string: %v", i, s) + } + } + } + + missingScopePresent := func(t *testing.T, parsed map[string]any) { + t.Helper() + data, ok := parsed["data"].(map[string]any) + if !ok { + t.Fatalf("parsed.data missing or wrong type: %v", parsed["data"]) + } + scope, ok := data["missingScope"].(string) + if !ok || scope == "" { + t.Fatalf("parsed.data.missingScope missing or not a string: %v", data["missingScope"]) + } + } + + agentCodeFieldPresent := func(t *testing.T, parsed map[string]any) { + t.Helper() + data, ok := parsed["data"].(map[string]any) + if !ok { + t.Fatalf("parsed.data missing or wrong type: %v", parsed["data"]) + } + if _, ok := data["agentCode"].(string); !ok { + t.Fatalf("parsed.data.agentCode missing or not a string: %v", data["agentCode"]) + } + } + + cases := []struct { + name string + code string + body map[string]any + extract assertFn + }{ + { + name: "PAT_NO_PERMISSION generic baseline", + code: "PAT_NO_PERMISSION", + body: map[string]any{ + "success": false, + "code": "PAT_NO_PERMISSION", + "data": map[string]any{ + "requiredScopes": []any{"aitable.record:read"}, + "grantOptions": []any{"session", "permanent"}, + "displayName": "阅读多维表记录", + "productName": "AI 表格", + }, + }, + extract: requiredScopesPresent, + }, + { + name: "PAT_LOW_RISK_NO_PERMISSION", + code: "PAT_LOW_RISK_NO_PERMISSION", + body: map[string]any{ + "success": false, + "code": "PAT_LOW_RISK_NO_PERMISSION", + "data": map[string]any{ + "requiredScopes": []any{"aitable.record:read"}, + "grantOptions": []any{"session", "permanent"}, + "displayName": "阅读多维表记录", + "productName": "AI 表格", + }, + }, + extract: requiredScopesPresent, + }, + { + name: "PAT_MEDIUM_RISK_NO_PERMISSION", + code: "PAT_MEDIUM_RISK_NO_PERMISSION", + body: map[string]any{ + "success": false, + "code": "PAT_MEDIUM_RISK_NO_PERMISSION", + "data": map[string]any{ + "requiredScopes": []any{"chat.group:write"}, + "grantOptions": []any{"session", "permanent"}, + "displayName": "发送群消息", + "productName": "群聊", + }, + }, + extract: requiredScopesPresent, + }, + { + name: "PAT_HIGH_RISK_NO_PERMISSION", + code: "PAT_HIGH_RISK_NO_PERMISSION", + body: map[string]any{ + "success": false, + "code": "PAT_HIGH_RISK_NO_PERMISSION", + "data": map[string]any{ + "requiredScopes": []any{"finance.invoice:write"}, + "grantOptions": []any{"once"}, + "displayName": "开具发票", + "productName": "财务", + "authRequestId": "auth-req-xxxx", + }, + }, + extract: requiredScopesPresent, + }, + { + name: "PAT_SCOPE_AUTH_REQUIRED", + code: "PAT_SCOPE_AUTH_REQUIRED", + body: map[string]any{ + "success": false, + "code": "PAT_SCOPE_AUTH_REQUIRED", + "data": map[string]any{ + "missingScope": "Contact.User.Read", + "hint": "run `dws auth login --scope Contact.User.Read`", + }, + }, + extract: missingScopePresent, + }, + { + name: "AGENT_CODE_NOT_EXISTS", + code: "AGENT_CODE_NOT_EXISTS", + body: map[string]any{ + "success": false, + "code": "AGENT_CODE_NOT_EXISTS", + "data": map[string]any{ + "agentCode": "agt-unknown", + "hint": "请检查 DINGTALK_DWS_AGENTCODE / agent 注册", + }, + }, + extract: agentCodeFieldPresent, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + patErr := errpkg.ClassifyPatAuthCheck(tc.body) + if patErr == nil { + t.Fatalf("ClassifyPatAuthCheck(%s) returned nil; expected *PATError", tc.code) + } + if patErr.ExitCode() != errpkg.ExitCodePermission { + t.Fatalf("ExitCode() = %d, want %d (PAT exit_code MUST be 4)", + patErr.ExitCode(), errpkg.ExitCodePermission) + } + + raw := patErr.RawStderr() + if raw == "" { + t.Fatalf("RawStderr() is empty; expected single-line JSON") + } + if strings.ContainsAny(raw, "\n\r") { + t.Fatalf("RawStderr() MUST be single-line (no \\n or \\r); got:\n%q", raw) + } + if strings.HasPrefix(raw, " ") || strings.HasPrefix(raw, "\t") { + t.Fatalf("RawStderr() MUST not have leading whitespace; got: %q", raw) + } + + var parsed map[string]any + if err := json.Unmarshal([]byte(raw), &parsed); err != nil { + t.Fatalf("json.Unmarshal(RawStderr()) error = %v; raw=%q", err, raw) + } + + if success, ok := parsed["success"].(bool); !ok || success { + t.Fatalf("parsed.success = %v, want false", parsed["success"]) + } + gotCode, ok := parsed["code"].(string) + if !ok { + t.Fatalf("parsed.code missing or not a string: %v", parsed["code"]) + } + if gotCode != tc.code { + t.Fatalf("parsed.code = %q, want %q", gotCode, tc.code) + } + + tc.extract(t, parsed) + }) + } +} + +// TestPATStderrJSON_CodeEnumFrozen pins the exact code enum frozen for +// PAT-family errors (see patNoPermissionCodes / patAuthRequiredCodes in +// internal/errors/pat.go). If a future refactor accidentally renames a +// code (e.g. drops the risk-tier prefix) the classifier will stop +// producing a *PATError and this table-driven loop will fail with a +// clear message — preventing a silent break of the host integration +// contract. +func TestPATStderrJSON_CodeEnumFrozen(t *testing.T) { + t.Parallel() + + frozen := []string{ + "PAT_NO_PERMISSION", + "PAT_LOW_RISK_NO_PERMISSION", + "PAT_MEDIUM_RISK_NO_PERMISSION", + "PAT_HIGH_RISK_NO_PERMISSION", + "PAT_SCOPE_AUTH_REQUIRED", + "AGENT_CODE_NOT_EXISTS", + } + + for _, code := range frozen { + code := code + t.Run(code, func(t *testing.T) { + t.Parallel() + body := map[string]any{ + "success": false, + "code": code, + "data": map[string]any{"requiredScopes": []any{"stub.entity:read"}}, + } + if patErr := errpkg.ClassifyPatAuthCheck(body); patErr == nil { + t.Fatalf("ClassifyPatAuthCheck(%q) returned nil — code was removed from the SSOT-frozen enum", code) + } + }) + } +}