diff --git a/extension/keysigner/keysigner.go b/extension/keysigner/keysigner.go index b0274e65e..fc2c123e3 100644 --- a/extension/keysigner/keysigner.go +++ b/extension/keysigner/keysigner.go @@ -74,7 +74,6 @@ func AlgForKey(pub crypto.PublicKey) (string, error) { // EncodePublicKey marshals pub to PKIX DER and base64-encodes it (std encoding), // matching the public-key form the registration backend binds to the app. -// Ported from the tee-test reference implementation. func EncodePublicKey(pub crypto.PublicKey) (string, error) { der, err := x509.MarshalPKIXPublicKey(pub) if err != nil { diff --git a/extension/keysigner/signer_keychain_darwin.go b/extension/keysigner/signer_keychain_darwin.go index 45f8662af..a852564c3 100644 --- a/extension/keysigner/signer_keychain_darwin.go +++ b/extension/keysigner/signer_keychain_darwin.go @@ -4,7 +4,6 @@ // SPDX-License-Identifier: MIT // macOS non-exportable Keychain signer (build tag `keychain_signer`). -// Ported from github.com/JackZhao10086/tee-test. // // It does NOT use the Secure Enclave / hardware TEE (which would require // code-signing entitlements that are unfriendly to open source). Instead it @@ -193,8 +192,13 @@ import ( "path/filepath" "strings" "unsafe" + + "github.com/larksuite/cli/internal/vfs" ) +// securityBin is invoked by absolute path so a poisoned PATH cannot hijack it. +const securityBin = "/usr/bin/security" + // keychainSigner implements Signer using a macOS non-exportable Keychain key. type keychainSigner struct{} @@ -277,12 +281,12 @@ func createKeychainKey(label string) (crypto.PublicKey, error) { } appLabel := sha1.Sum(x509.MarshalPKCS1PublicKey(&privateKey.PublicKey)) - pemFile, err := os.CreateTemp("", "lark-keysigner-*.pem") + pemFile, err := vfs.CreateTemp("", "lark-keysigner-*.pem") if err != nil { return nil, fmt.Errorf("keysigner: temp key file: %w", err) } pemPath := pemFile.Name() - defer os.Remove(pemPath) + defer vfs.Remove(pemPath) if err := pemFile.Chmod(0600); err != nil { pemFile.Close() return nil, err @@ -297,7 +301,7 @@ func createKeychainKey(label string) (crypto.PublicKey, error) { return nil, err } - executable, err := os.Executable() + executable, err := vfs.Executable() if err != nil { return nil, fmt.Errorf("keysigner: resolve executable: %w", err) } @@ -306,9 +310,9 @@ func createKeychainKey(label string) (crypto.PublicKey, error) { return nil, err } // -x: import as NON-EXTRACTABLE; the software copy (pemPath) is then removed. - importCmd := exec.Command("security", "import", pemPath, "-k", keychain, "-t", "priv", "-f", "openssl", "-x", "-A", "-T", executable) + importCmd := exec.Command(securityBin, "import", pemPath, "-k", keychain, "-t", "priv", "-f", "openssl", "-x", "-A", "-T", executable) if out, err := importCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("keysigner: import non-extractable key: %w: %s", err, string(out)) + return nil, fmt.Errorf("keysigner: import non-extractable key: %w: %s", err, summarizeCmdOutput(out)) } if err := setKeychainKeyLabel(appLabel[:], keychain, label); err != nil { return nil, err @@ -369,7 +373,7 @@ func readKeyMetadata(label string) (*keyMetadata, error) { if err != nil { return nil, err } - data, err := os.ReadFile(path) + data, err := vfs.ReadFile(path) if err != nil { return nil, err // preserves os.ErrNotExist for EnsureKey } @@ -381,14 +385,14 @@ func readKeyMetadata(label string) (*keyMetadata, error) { } func writeKeyMetadata(path string, md keyMetadata) error { - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + if err := vfs.MkdirAll(filepath.Dir(path), 0700); err != nil { return err } data, err := json.MarshalIndent(md, "", " ") if err != nil { return err } - return os.WriteFile(path, data, 0600) + return vfs.WriteFile(path, data, 0600) } func ensureKeychain() (string, error) { @@ -400,11 +404,11 @@ func ensureKeychain() (string, error) { if err != nil { return "", err } - if _, err := os.Stat(keychainPath); err != nil { + if _, err := vfs.Stat(keychainPath); err != nil { if !os.IsNotExist(err) { return "", fmt.Errorf("keysigner: stat keychain: %w", err) } - if err := os.MkdirAll(filepath.Dir(keychainPath), 0700); err != nil { + if err := vfs.MkdirAll(filepath.Dir(keychainPath), 0700); err != nil { return "", err } for _, args := range [][]string{ @@ -412,8 +416,8 @@ func ensureKeychain() (string, error) { {"set-keychain-settings", keychainPath}, {"unlock-keychain", "-p", password, keychainPath}, } { - if out, err := exec.Command("security", args...).CombinedOutput(); err != nil { - return "", fmt.Errorf("keysigner: security %s: %w: %s", args[0], err, string(out)) + if out, err := exec.Command(securityBin, args...).CombinedOutput(); err != nil { + return "", fmt.Errorf("keysigner: security %s: %w: %s", args[0], err, summarizeCmdOutput(out)) } } } @@ -442,7 +446,7 @@ func keychainPassword() (string, error) { return "", err } path := filepath.Join(dir, "keychain.pass") - if data, err := os.ReadFile(path); err == nil { + if data, err := vfs.ReadFile(path); err == nil { if pw := strings.TrimSpace(string(data)); pw != "" { return pw, nil } @@ -455,10 +459,10 @@ func keychainPassword() (string, error) { return "", err } pw := hex.EncodeToString(buf) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + if err := vfs.MkdirAll(filepath.Dir(path), 0700); err != nil { return "", err } - if err := os.WriteFile(path, []byte(pw+"\n"), 0600); err != nil { + if err := vfs.WriteFile(path, []byte(pw+"\n"), 0600); err != nil { return "", err } return pw, nil @@ -473,6 +477,20 @@ func keyMetadataPath(label string) (string, error) { return filepath.Join(dir, "keys", hex.EncodeToString(id[:])+".json"), nil } +// summarizeCmdOutput bounds external command output before it is embedded in +// an error: first line only, capped at 200 chars. +func summarizeCmdOutput(out []byte) string { + s := strings.TrimSpace(string(out)) + if i := strings.IndexByte(s, '\n'); i >= 0 { + s = strings.TrimSpace(s[:i]) + } + const maxLen = 200 + if len(s) > maxLen { + s = s[:maxLen] + "..." + } + return s +} + func keychainError(operation string, status int) error { switch status { case -25299: diff --git a/internal/auth/app_registration.go b/internal/auth/app_registration.go index 712c4e741..0021c5c5c 100644 --- a/internal/auth/app_registration.go +++ b/internal/auth/app_registration.go @@ -189,7 +189,14 @@ func RequestAppRegistration(httpClient *http.Client, brand core.LarkBrand, opts if base == "" { base = ep.Open + "/page/launcher" } - verificationUriComplete = fmt.Sprintf("%s?user_code=%s", base, userCode) + // The server may return verification_uri with its own query (e.g. + // client_id when registering against an existing app), so join with + // the same ?/& logic as BuildVerificationURL. + sep := "?" + if strings.Contains(base, "?") { + sep = "&" + } + verificationUriComplete = base + sep + "user_code=" + url.QueryEscape(userCode) } return &AppRegistrationResponse{ diff --git a/internal/auth/app_registration_test.go b/internal/auth/app_registration_test.go index c697a43f6..c490df048 100644 --- a/internal/auth/app_registration_test.go +++ b/internal/auth/app_registration_test.go @@ -103,6 +103,38 @@ func TestRequestAppRegistration_BeginDefaultsToClientSecret(t *testing.T) { } } +func TestRequestAppRegistration_VerificationURICompleteFallback(t *testing.T) { + cases := []struct { + name string + resp string + want string + }{ + { + name: "bare verification_uri", + resp: `{"device_code":"dc","user_code":"uc","verification_uri":"https://example/verify","expires_in":300,"interval":5}`, + want: "https://example/verify?user_code=uc", + }, + { + name: "verification_uri with existing query", + resp: `{"device_code":"dc","user_code":"uc","verification_uri":"https://example/verify?client_id=cli_x","expires_in":300,"interval":5}`, + want: "https://example/verify?client_id=cli_x&user_code=uc", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var body url.Values + hc := captureClient(&body, tc.resp) + got, err := RequestAppRegistration(hc, core.BrandFeishu, AppRegistrationBeginOptions{}, nil) + if err != nil { + t.Fatal(err) + } + if got.VerificationUriComplete != tc.want { + t.Errorf("VerificationUriComplete = %q, want %q", got.VerificationUriComplete, tc.want) + } + }) + } +} + func TestParseAuthMethods(t *testing.T) { if got := parseAuthMethods([]interface{}{"private_key_jwt", "client_secret"}); len(got) != 2 || got[0] != "private_key_jwt" { t.Errorf("array form = %v", got) diff --git a/internal/auth/jwt/jwt_test.go b/internal/auth/jwt/jwt_test.go index 0b7dfaf8d..049158451 100644 --- a/internal/auth/jwt/jwt_test.go +++ b/internal/auth/jwt/jwt_test.go @@ -117,6 +117,23 @@ func TestBuildSignedJWT_AlgMismatch(t *testing.T) { } } +func TestBuildSignedJWT_MarshalErrors(t *testing.T) { + f := newFakeSigner(t) + ctx := context.Background() + + _, err := buildSignedJWT(ctx, f, keysigner.KeyRef{}, keysigner.AlgES256, + map[string]any{"bad": func() {}}, nil) + if err == nil || !strings.Contains(err.Error(), "jwt: marshal header") { + t.Fatalf("header marshal error = %v, want prefix %q", err, "jwt: marshal header") + } + + _, err = buildSignedJWT(ctx, f, keysigner.KeyRef{}, keysigner.AlgES256, + nil, map[string]any{"bad": make(chan int)}) + if err == nil || !strings.Contains(err.Error(), "jwt: marshal claims") { + t.Fatalf("claims marshal error = %v, want prefix %q", err, "jwt: marshal claims") + } +} + func TestSignClientAssertion(t *testing.T) { f := newFakeSigner(t) now := time.Unix(1700000000, 0)