From ef7e0bf0d31bdc9ce9e5496c64e420d625f9d9ff Mon Sep 17 00:00:00 2001 From: piekstra Date: Mon, 19 Oct 2026 09:25:50 -0400 Subject: [PATCH 1/2] feat(apps): manage Slack apps through their manifests Add `slck apps` with get, validate, diff, create, update and delete on the apps.manifest.* methods, authenticated with an app configuration token pair stored under a new keyring key, app_config_token. The pair enters through `slck set-credential --key app_config_token --stdin` as {"token", "refresh_token", "exp"} JSON. slck rotates it with tooling.tokens.rotate when the access token is within five minutes of expiry or Slack rejects it, saving the new pair before using it, and `config show` reports its expiry and team without the tokens. `apps diff` exits 0 when the live manifest matches the file, 1 when it differs and 2 when it could not compare; the root command now honors an exit code an error carries. `apps create` never prints the new app's secrets and writes them only to a new 0600 file when asked. --- README.md | 48 ++- integration-tests.md | 47 +++ internal/apptoken/apptoken.go | 74 ++++ internal/apptoken/apptoken_test.go | 54 +++ internal/client/apps.go | 182 +++++++++ internal/cmd/apps/apps.go | 380 ++++++++++++++++++ internal/cmd/apps/apps_test.go | 324 +++++++++++++++ internal/cmd/apps/diff.go | 93 +++++ internal/cmd/apps/manifest.go | 57 +++ internal/cmd/apps/session.go | 95 +++++ .../cmd/apps/testdata/create-response.json | 1 + .../testdata/invalid-manifest-response.json | 1 + .../cmd/apps/testdata/manifest-export.json | 1 + .../apps/testdata/manifest-more-scopes.json | 1 + internal/cmd/apps/testdata/manifest.json | 25 ++ internal/cmd/apps/testdata/manifest.yaml | 16 + internal/cmd/apps/testdata/ok-response.json | 1 + .../cmd/apps/testdata/rotate-response.json | 1 + .../apps/testdata/token-expired-response.json | 1 + .../cmd/apps/testdata/update-response.json | 1 + internal/cmd/config/show.go | 27 +- internal/cmd/root/root.go | 25 +- internal/cmd/root/root_test.go | 28 ++ internal/cmd/setcred/setcred.go | 29 +- internal/cmd/setcred/setcred_test.go | 33 ++ internal/keychain/keychain.go | 24 +- internal/noleak/noleak_test.go | 30 ++ 27 files changed, 1586 insertions(+), 13 deletions(-) create mode 100644 internal/apptoken/apptoken.go create mode 100644 internal/apptoken/apptoken_test.go create mode 100644 internal/client/apps.go create mode 100644 internal/cmd/apps/apps.go create mode 100644 internal/cmd/apps/apps_test.go create mode 100644 internal/cmd/apps/diff.go create mode 100644 internal/cmd/apps/manifest.go create mode 100644 internal/cmd/apps/session.go create mode 100644 internal/cmd/apps/testdata/create-response.json create mode 100644 internal/cmd/apps/testdata/invalid-manifest-response.json create mode 100644 internal/cmd/apps/testdata/manifest-export.json create mode 100644 internal/cmd/apps/testdata/manifest-more-scopes.json create mode 100644 internal/cmd/apps/testdata/manifest.json create mode 100644 internal/cmd/apps/testdata/manifest.yaml create mode 100644 internal/cmd/apps/testdata/ok-response.json create mode 100644 internal/cmd/apps/testdata/rotate-response.json create mode 100644 internal/cmd/apps/testdata/token-expired-response.json create mode 100644 internal/cmd/apps/testdata/update-response.json diff --git a/README.md b/README.md index 8dda539..474af6a 100644 --- a/README.md +++ b/README.md @@ -854,6 +854,52 @@ slck canvas delete F12345ABC > **Note:** Canvas commands require additional Slack app scopes not included in the default manifest. See [Slack Canvas API docs](https://docs.slack.dev/surfaces/canvases/) for required scopes. +### Apps + +Manage Slack apps through their manifests, so an app's configuration can live in version control and be reviewed like code. + +```bash +# Store an app configuration token pair (generate one at api.slack.com/apps, +# "Your App Configuration Tokens"); the JSON is +# {"token": "xoxe.xoxp-...", "refresh_token": "xoxe-...", "exp": } + | slck set-credential --key app_config_token --stdin + +# Export an app's manifest +slck apps get A0123456789 > app-manifest.json + +# Check a manifest, optionally as an update to an existing app +slck apps validate --file app-manifest.json --app A0123456789 + +# See what an update would change (exit 0 same, 1 different, 2 could not compare) +slck apps diff A0123456789 --file app-manifest.json + +# Apply it +slck apps update A0123456789 --file app-manifest.json + +# Create an app; its secrets go only to a new 0600 file +slck apps create --file app-manifest.json --credentials-file app-credentials.json + +# Permanently delete an app (prompts; --force skips the prompt) +slck apps delete A0123456789 +``` + +#### Apps Command Reference + +| Command | Flags | Description | +|---------|-------|-------------| +| `get ` | `--to-file` | Print an app's manifest as indented JSON with sorted keys | +| `validate` | `--file`, `--app` | Check a manifest with `apps.manifest.validate` | +| `diff ` | `--file` | Compare the live manifest with a file, one line per difference | +| `create` | `--file`, `--credentials-file` | Create an app; prints its ID and install URL | +| `update ` | `--file` | Replace an app's manifest; says when a reinstall is needed | +| `delete ` | `--force` | Permanently delete an app | + +Manifest files are JSON, or YAML when the name ends in `.yaml` or `.yml`. + +The apps commands authenticate with the app configuration token, not the bot or user token. slck rotates the pair with `tooling.tokens.rotate` when the access token is within five minutes of expiry or Slack rejects it, and saves the new pair to the keyring before using it. `slck config show` shows when the access token expires and which team it belongs to, never the tokens. + +Installing an app to a workspace and reading its bot token need a person in a browser; these commands do neither. + ### Identity ```bash @@ -884,7 +930,7 @@ Ingress is at the top level, not under `config`: | Command | Flags | Description | |---------|-------|-------------| | `slck init` | `--bot-token-from-env`, `--user-token-from-env`, `--bot-token-stdin`, `--overwrite`, `--no-verify` | Guided setup; stores into the keyring | -| `slck set-credential` | `--key`, `--stdin`, `--from-env`, `--ref` | Set one credential (stdin/env only) | +| `slck set-credential` | `--key`, `--stdin`, `--from-env`, `--ref` | Set one credential (stdin/env only): `bot_token`, `user_token` or `app_config_token` | `config` subcommands (none accept a secret value): diff --git a/integration-tests.md b/integration-tests.md index 3809848..b7ffe54 100644 --- a/integration-tests.md +++ b/integration-tests.md @@ -641,6 +641,52 @@ Using **CANVAS_ID₁** from step 10.1: --- +## Part 11: Apps Tests + +Needs an app configuration token pair for the test workspace, generated at api.slack.com/apps under "Your App Configuration Tokens", and the ID of an app in that workspace (APP_ID). The read-only steps change nothing; steps 11.4 and 11.5 create and delete a throwaway app. + +### 11.1 Store the Token Pair + +| Step | Command | Expected | +|------|---------|----------| +| 1 | `printf '{"token":"%s","refresh_token":"%s","exp":%s}' "$ACCESS" "$REFRESH" "$EXP" \| slck set-credential --key app_config_token --stdin` | "Stored app_config_token in ..." with no token in the output | +| 2 | `slck config show` | `app_config_token: present, access token expires ...` | + +### 11.2 Read-Only + +| Step | Command | Expected | +|------|---------|----------| +| 1 | `slck apps get --to-file app.json` | "Wrote the manifest of ..." | +| 2 | `slck apps validate --file app.json --app ` | "Valid: app.json", exit 0 | +| 3 | `slck apps diff --file app.json` | "No differences", exit 0 | +| 4 | Add a scope to `app.json`, then `slck apps diff --file app.json` | One `~ /oauth_config/scopes/bot` line, exit 1 | +| 5 | `slck apps diff --file missing.json` | Read error, exit 2 | + +### 11.3 Rotation + +| Step | Command | Expected | +|------|---------|----------| +| 1 | Store a pair whose `exp` is in the past (11.1 with `EXP` set to an earlier time), then `slck apps get ` | Manifest printed | +| 2 | `slck config show` | A new expiry about 12 hours out and the team ID | + +### 11.4 Create and Update a Throwaway App + +| Step | Command | Expected | +|------|---------|----------| +| 1 | `slck apps create --file throwaway.json --credentials-file creds.json` | "Created app A..." and an install URL; no secret in the output; `creds.json` is mode 0600 | +| 2 | `slck apps update --file throwaway.json` | "Updated app ..." | +| 3 | Add a scope, then `slck apps update --file throwaway.json` | Also "reinstall the app ..." | + +### 11.5 Cleanup: Delete the Throwaway App + +| Step | Command | Expected | +|------|---------|----------| +| 1 | `slck apps delete < /dev/null` | Refuses: stdin is not a terminal, pass --force | +| 2 | `slck apps delete --force` | "Deleted app ..." | +| 3 | `rm creds.json` | | + +--- + ## Troubleshooting | Error | Cause | Solution | @@ -703,3 +749,4 @@ slck messages history $TEST_CHANNEL_ID --limit 5 | Part 3B (Search) | Search test message TS₃ | `messages delete` | | Part 5 (Destructive) | Test channels | `channels archive` | | Part 10 (Canvas) | Test canvases CANVAS_ID₁–₃ | `canvas delete` | +| Part 11 (Apps) | Throwaway app NEW_APP_ID, `creds.json` | `apps delete --force`, `rm` | diff --git a/internal/apptoken/apptoken.go b/internal/apptoken/apptoken.go new file mode 100644 index 0000000..b94d62c --- /dev/null +++ b/internal/apptoken/apptoken.go @@ -0,0 +1,74 @@ +// Package apptoken models a Slack app configuration token pair: the +// short-lived access token the apps.manifest.* methods accept, the refresh +// token that tooling.tokens.rotate exchanges for a new pair, and the access +// token's expiry. +// +// The pair is stored in the keyring as one JSON value under +// keychain.KeyAppConfigToken, so a rotation replaces both tokens in a single +// write. The stored shape is a superset of the ingress contract. +package apptoken + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + "time" +) + +// RotateWithin is how close to expiry an access token is rotated before use. +// Slack issues access tokens for 12 hours. +const RotateWithin = 5 * time.Minute + +// Pair is a stored app configuration token pair. Token and RefreshToken are +// secrets and are never printed. +type Pair struct { + Token string `json:"token"` + RefreshToken string `json:"refresh_token"` + // Exp is the access token's expiry in Unix seconds. + Exp int64 `json:"exp"` + // TeamID and UserID are recorded from the first rotation; the ingress + // contract does not carry them. + TeamID string `json:"team_id,omitempty"` + UserID string `json:"user_id,omitempty"` +} + +// Parse validates and decodes an ingress value or a stored value. The +// ingress contract is exactly {"token", "refresh_token", "exp"}, the field +// names tooling.tokens.rotate answers with. Errors never include a token. +func Parse(raw string) (Pair, error) { + dec := json.NewDecoder(strings.NewReader(raw)) + dec.DisallowUnknownFields() + var p Pair + if err := dec.Decode(&p); err != nil { + return Pair{}, errors.New(`app configuration token must be a JSON object {"token": "...", "refresh_token": "...", "exp": }`) + } + if !strings.HasPrefix(p.Token, "xoxe.") { + return Pair{}, errors.New(`app configuration token: "token" is not an app configuration access token (xoxe.*)`) + } + if !strings.HasPrefix(p.RefreshToken, "xoxe-") { + return Pair{}, errors.New(`app configuration token: "refresh_token" is not an app configuration refresh token (xoxe-*)`) + } + if p.Exp <= 0 { + return Pair{}, errors.New(`app configuration token: "exp" must be the access token's expiry in Unix seconds`) + } + return p, nil +} + +// Encode returns the stored JSON value. +func (p Pair) Encode() (string, error) { + b, err := json.Marshal(p) + if err != nil { + return "", fmt.Errorf("encode app configuration token: %w", err) + } + return string(b), nil +} + +// Expiry returns the access token's expiry. +func (p Pair) Expiry() time.Time { return time.Unix(p.Exp, 0) } + +// NeedsRotation reports whether the access token expires within RotateWithin +// of now. +func (p Pair) NeedsRotation(now time.Time) bool { + return !now.Add(RotateWithin).Before(p.Expiry()) +} diff --git a/internal/apptoken/apptoken_test.go b/internal/apptoken/apptoken_test.go new file mode 100644 index 0000000..793049a --- /dev/null +++ b/internal/apptoken/apptoken_test.go @@ -0,0 +1,54 @@ +package apptoken + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + access = "xoxe.xoxp-1-access-canary-3f9a" + refresh = "xoxe-1-refresh-canary-81c2" +) + +func TestParseAcceptsTheIngressContract(t *testing.T) { + p, err := Parse(`{"token": "` + access + `", "refresh_token": "` + refresh + `", "exp": 1790043200}`) + require.NoError(t, err) + assert.Equal(t, access, p.Token) + assert.Equal(t, refresh, p.RefreshToken) + assert.Equal(t, time.Unix(1790043200, 0), p.Expiry()) +} + +func TestParseRoundTripsAStoredPair(t *testing.T) { + stored, err := Pair{Token: access, RefreshToken: refresh, Exp: 1790043200, TeamID: "T0TEST", UserID: "U0TEST"}.Encode() + require.NoError(t, err) + p, err := Parse(stored) + require.NoError(t, err) + assert.Equal(t, "T0TEST", p.TeamID) +} + +func TestParseRejectionsNeverIncludeTheValue(t *testing.T) { + for name, raw := range map[string]string{ + "not json": access, + "unknown field": `{"token": "` + access + `", "refresh_token": "` + refresh + `", "exp": 1, "access": "x"}`, + "bot token": `{"token": "xoxb-1-canary", "refresh_token": "` + refresh + `", "exp": 1}`, + "swapped tokens": `{"token": "` + refresh + `", "refresh_token": "` + access + `", "exp": 1}`, + "missing expiry": `{"token": "` + access + `", "refresh_token": "` + refresh + `"}`, + } { + t.Run(name, func(t *testing.T) { + _, err := Parse(raw) + require.Error(t, err) + assert.NotContains(t, err.Error(), "canary") + }) + } +} + +func TestNeedsRotation(t *testing.T) { + exp := time.Unix(1790043200, 0) + p := Pair{Exp: exp.Unix()} + assert.False(t, p.NeedsRotation(exp.Add(-time.Hour))) + assert.True(t, p.NeedsRotation(exp.Add(-RotateWithin))) + assert.True(t, p.NeedsRotation(exp.Add(time.Hour))) +} diff --git a/internal/client/apps.go b/internal/client/apps.go new file mode 100644 index 0000000..9a3012a --- /dev/null +++ b/internal/client/apps.go @@ -0,0 +1,182 @@ +package client + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +// AppsClient calls the app manifest methods (apps.manifest.*) and +// tooling.tokens.rotate. Unlike Client it carries no token of its own: the +// manifest methods take an app configuration access token, which rotates, +// so each call is given the current one. +type AppsClient struct { + httpClient *http.Client + baseURL string +} + +// NewAppsClient returns an AppsClient for the Slack API. +func NewAppsClient() *AppsClient { + return NewAppsClientWithConfig(defaultBaseURL, nil) +} + +// NewAppsClientWithConfig returns an AppsClient for baseURL; tests pass an +// httptest server. +func NewAppsClientWithConfig(baseURL string, httpClient *http.Client) *AppsClient { + if httpClient == nil { + httpClient = &http.Client{Timeout: 30 * time.Second} + } + return &AppsClient{httpClient: httpClient, baseURL: baseURL} +} + +// ManifestError is one problem apps.manifest.validate, create or update +// found in a manifest. Pointer is a JSON pointer into the manifest. +type ManifestError struct { + Message string `json:"message"` + Pointer string `json:"pointer"` +} + +// APIError is a Slack answer with ok=false. Errors carries the manifest +// problems Slack lists for invalid_manifest. +type APIError struct { + Method string + Code string + Errors []ManifestError +} + +func (e *APIError) Error() string { + msg := fmt.Sprintf("slack API error: %s (%s)", e.Code, e.Method) + for _, m := range e.Errors { + msg += fmt.Sprintf("\n %s: %s", m.Pointer, m.Message) + } + return msg +} + +// ExpiredToken reports whether the error means the access token no longer +// works, so a rotation may recover it. +func (e *APIError) ExpiredToken() bool { + return e.Code == "token_expired" || e.Code == "invalid_auth" +} + +// AppCredentials are the secrets apps.manifest.create returns for a new +// app. They are written only to a file the caller names, never printed. +type AppCredentials struct { + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + VerificationToken string `json:"verification_token"` + SigningSecret string `json:"signing_secret"` +} + +// CreatedApp is apps.manifest.create's answer. +type CreatedApp struct { + AppID string `json:"app_id"` + Credentials AppCredentials `json:"credentials"` + OAuthAuthorizeURL string `json:"oauth_authorize_url"` +} + +// RotatedToken is tooling.tokens.rotate's answer. +type RotatedToken struct { + Token string `json:"token"` + RefreshToken string `json:"refresh_token"` + TeamID string `json:"team_id"` + UserID string `json:"user_id"` + Exp int64 `json:"exp"` +} + +// ExportManifest returns an app's manifest exactly as Slack sends it. +func (c *AppsClient) ExportManifest(token, appID string) (json.RawMessage, error) { + var resp struct { + Manifest json.RawMessage `json:"manifest"` + } + if err := c.call("apps.manifest.export", token, url.Values{"app_id": {appID}}, &resp); err != nil { + return nil, err + } + return resp.Manifest, nil +} + +// ValidateManifest checks a manifest, against an existing app when appID is +// set. A manifest with problems returns an *APIError listing them. +func (c *AppsClient) ValidateManifest(token string, manifest json.RawMessage, appID string) error { + params := url.Values{"manifest": {string(manifest)}} + if appID != "" { + params.Set("app_id", appID) + } + return c.call("apps.manifest.validate", token, params, nil) +} + +// CreateApp creates an app from a manifest. +func (c *AppsClient) CreateApp(token string, manifest json.RawMessage) (CreatedApp, error) { + var resp CreatedApp + err := c.call("apps.manifest.create", token, url.Values{"manifest": {string(manifest)}}, &resp) + return resp, err +} + +// UpdateApp replaces an app's manifest and reports whether its scopes +// changed, in which case the app must be reinstalled for them to apply. +func (c *AppsClient) UpdateApp(token, appID string, manifest json.RawMessage) (bool, error) { + var resp struct { + PermissionsUpdated bool `json:"permissions_updated"` + } + err := c.call("apps.manifest.update", token, url.Values{"app_id": {appID}, "manifest": {string(manifest)}}, &resp) + return resp.PermissionsUpdated, err +} + +// DeleteApp permanently deletes an app. +func (c *AppsClient) DeleteApp(token, appID string) error { + return c.call("apps.manifest.delete", token, url.Values{"app_id": {appID}}, nil) +} + +// RotateToken exchanges a refresh token for a new configuration token pair. +// It takes no access token. +func (c *AppsClient) RotateToken(refreshToken string) (RotatedToken, error) { + var resp RotatedToken + err := c.call("tooling.tokens.rotate", "", url.Values{"refresh_token": {refreshToken}}, &resp) + return resp, err +} + +// call POSTs form-encoded params, which every method here accepts, and +// decodes the answer into out when it is ok. +func (c *AppsClient) call(method, token string, params url.Values, out any) error { + req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/%s", c.baseURL, method), strings.NewReader(params.Encode())) + if err != nil { + return err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("%s: %w", method, err) + } + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if err != nil { + return fmt.Errorf("%s: %w", method, err) + } + if resp.StatusCode == http.StatusTooManyRequests { + return fmt.Errorf("%s: rate limited by Slack; retry after %s seconds", method, resp.Header.Get("Retry-After")) + } + var status struct { + OK bool `json:"ok"` + Error string `json:"error"` + Errors []ManifestError `json:"errors"` + } + if err := json.Unmarshal(body, &status); err != nil { + return fmt.Errorf("%s: unexpected response (HTTP %d)", method, resp.StatusCode) + } + if !status.OK { + return &APIError{Method: method, Code: status.Error, Errors: status.Errors} + } + if out == nil { + return nil + } + if err := json.Unmarshal(body, out); err != nil { + return fmt.Errorf("%s: decode response: %w", method, err) + } + return nil +} diff --git a/internal/cmd/apps/apps.go b/internal/cmd/apps/apps.go new file mode 100644 index 0000000..589dcbf --- /dev/null +++ b/internal/cmd/apps/apps.go @@ -0,0 +1,380 @@ +// Package apps implements `slck apps`: managing Slack apps through their +// manifests (apps.manifest.*), authenticated with an app configuration +// token rather than the bot or user token. +package apps + +import ( + "bufio" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "strings" + + "github.com/spf13/cobra" + "golang.org/x/term" + + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/output" +) + +// exitError makes the root command exit with code instead of 1. A silent +// one prints nothing more, because the command already reported why. +type exitError struct { + code int + err error + silent bool +} + +func (e *exitError) Error() string { + if e.err == nil { + return "" + } + return e.err.Error() +} +func (e *exitError) Unwrap() error { return e.err } +func (e *exitError) ExitCode() int { return e.code } +func (e *exitError) Silent() bool { return e.silent } +func usageOrFailure(err error) error { return &exitError{code: 2, err: err} } + +// appsHints map apps.manifest.* error codes to what to do about them. +var appsHints = map[string]string{ + "invalid_auth": "Slack rejected the app configuration token even after rotating it. Generate a new pair at https://api.slack.com/apps and store it with 'slck set-credential --key app_config_token --stdin'.", + "token_expired": "Slack rejected the app configuration token even after rotating it. Generate a new pair at https://api.slack.com/apps and store it with 'slck set-credential --key app_config_token --stdin'.", + "invalid_app_id": "Check the app ID (it starts with A). The configuration token only reaches apps in its own workspace.", + "invalid_app": "Check the app ID (it starts with A). The configuration token only reaches apps in its own workspace.", + "invalid_manifest": "Fix the problems listed above and run 'slck apps validate' again.", +} + +// wrap names the operation and adds a hint for known apps.manifest.* errors. +func wrap(operation string, err error) error { + var apiErr *client.APIError + if errors.As(err, &apiErr) { + if hint, ok := appsHints[apiErr.Code]; ok { + return fmt.Errorf("%s: %w\nHint: %s", operation, err, hint) + } + } + return fmt.Errorf("%s: %w", operation, err) +} + +// quiet stops cobra from printing a runtime error and the usage text: once +// the arguments parsed, the root command prints the error alone, and a +// silent exitError (diff's "differs") prints nothing more. +func quiet(cmd *cobra.Command) { + cmd.SilenceUsage = true + cmd.SilenceErrors = true +} + +// NewCmd builds `slck apps`. +func NewCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "apps", + Short: "Manage Slack apps through their manifests", + Long: `Manage Slack apps through their manifests (apps.manifest.* methods). + +These commands authenticate with an app configuration token, not the bot or +user token. Generate one at https://api.slack.com/apps under "Your App +Configuration Tokens" and store the pair: + + | slck set-credential --key app_config_token --stdin + +The JSON is {"token": "xoxe.xoxp-...", "refresh_token": "xoxe-...", "exp": }. +slck rotates the pair with tooling.tokens.rotate when the access token is +within five minutes of expiry or Slack rejects it, and saves the new pair to +the keyring before using it. + +Installing an app and reading its bot token need a person in a browser; +these commands do not do either.`, + } + cmd.AddCommand(newGetCmd(), newValidateCmd(), newDiffCmd(), newCreateCmd(), newUpdateCmd(), newDeleteCmd()) + return cmd +} + +func newGetCmd() *cobra.Command { + var toFile string + cmd := &cobra.Command{ + Use: "get ", + Short: "Print an app's manifest", + Long: `Print an app's manifest as indented JSON with sorted keys, the form +the other apps commands read back. The manifest is the document itself, so +it is printed as JSON whatever --output says.`, + Example: ` slck apps get A0123456789 > app-manifest.json + slck apps get A0123456789 --to-file app-manifest.json`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runGet(newSession(), args[0], toFile) + }, + } + cmd.Flags().StringVar(&toFile, "to-file", "", "Write the manifest to this path instead of stdout") + return cmd +} + +func runGet(s *session, appID, toFile string) error { + var manifest json.RawMessage + err := s.do(func(token string) (err error) { + manifest, err = s.api.ExportManifest(token, appID) + return err + }) + if err != nil { + return wrap("export manifest of "+appID, err) + } + doc, err := indent(manifest) + if err != nil { + return err + } + if toFile == "" { + _, err = output.Writer.Write(doc) + return err + } + if err := os.WriteFile(toFile, doc, 0o644); err != nil { + return fmt.Errorf("write manifest: %w", err) + } + output.Printf("Wrote the manifest of %s to %s\n", appID, toFile) + return nil +} + +func newValidateCmd() *cobra.Command { + var file, appID string + cmd := &cobra.Command{ + Use: "validate --file ", + Short: "Check a manifest without changing anything", + Long: `Check a manifest file with apps.manifest.validate. With --app, Slack +also checks it as an update to that app. Exits 0 when the manifest is valid +and non-zero, listing each problem, when it is not.`, + Example: ` slck apps validate --file app-manifest.json + slck apps validate --file app-manifest.yaml --app A0123456789`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runValidate(newSession(), file, appID) + }, + } + cmd.Flags().StringVarP(&file, "file", "F", "", "Manifest file (.json, .yaml or .yml)") + cmd.Flags().StringVar(&appID, "app", "", "Validate as an update to this app") + return cmd +} + +func runValidate(s *session, file, appID string) error { + manifest, err := readManifest(file) + if err != nil { + return err + } + if err := s.do(func(token string) error { return s.api.ValidateManifest(token, manifest, appID) }); err != nil { + return wrap("validate "+file, err) + } + output.Printf("Valid: %s\n", file) + return nil +} + +func newDiffCmd() *cobra.Command { + var file string + cmd := &cobra.Command{ + Use: "diff --file ", + Short: "Compare an app's live manifest with a file", + Long: `Compare an app's live manifest with a manifest file, one line per +difference: "~" a value that differs, "-" a key only the live app has, "+" a +key only the file has. Paths are JSON pointers. + +Exit status: 0 when they match, 1 when they differ, 2 when the comparison +could not be made.`, + Example: ` slck apps diff A0123456789 --file app-manifest.json`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runDiff(newSession(), args[0], file) + }, + } + cmd.Flags().StringVarP(&file, "file", "F", "", "Manifest file (.json, .yaml or .yml)") + return cmd +} + +func runDiff(s *session, appID, file string) error { + manifest, err := readManifest(file) + if err != nil { + return usageOrFailure(err) + } + var live json.RawMessage + err = s.do(func(token string) (err error) { + live, err = s.api.ExportManifest(token, appID) + return err + }) + if err != nil { + return usageOrFailure(wrap("export manifest of "+appID, err)) + } + changes, err := diffManifests(live, manifest) + if err != nil { + return usageOrFailure(err) + } + if len(changes) == 0 { + output.Printf("No differences between %s and %s\n", appID, file) + return nil + } + for _, c := range changes { + output.Println(c.describe()) + } + return &exitError{code: 1, silent: true} +} + +func newCreateCmd() *cobra.Command { + var file, credentialsFile string + cmd := &cobra.Command{ + Use: "create --file ", + Short: "Create an app from a manifest", + Long: `Create an app from a manifest file and print its app ID and the URL +that installs it. + +Slack returns the new app's client secret, signing secret and verification +token once. They are never printed: --credentials-file writes them as JSON to +a new file readable only by you. Without it they are discarded, and the +Basic Information page of the app still shows them.`, + Example: ` slck apps create --file app-manifest.json --credentials-file app-credentials.json`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runCreate(newSession(), file, credentialsFile) + }, + } + cmd.Flags().StringVarP(&file, "file", "F", "", "Manifest file (.json, .yaml or .yml)") + cmd.Flags().StringVar(&credentialsFile, "credentials-file", "", "Write the new app's credentials to this new file (mode 0600)") + return cmd +} + +func runCreate(s *session, file, credentialsFile string) error { + manifest, err := readManifest(file) + if err != nil { + return err + } + // Open the credentials file first, so a bad path fails before an app + // exists whose secrets could not be kept. + var creds *os.File + if credentialsFile != "" { + creds, err = os.OpenFile(credentialsFile, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return fmt.Errorf("credentials file: %w", err) + } + defer func() { _ = creds.Close() }() + } + var app client.CreatedApp + err = s.do(func(token string) (err error) { + app, err = s.api.CreateApp(token, manifest) + return err + }) + if err != nil { + if creds != nil { + _ = creds.Close() + _ = os.Remove(credentialsFile) + } + return wrap("create app from "+file, err) + } + if creds != nil { + enc := json.NewEncoder(creds) + enc.SetIndent("", " ") + if err := enc.Encode(app.Credentials); err != nil { + return fmt.Errorf("created app %s but could not write its credentials: %w", app.AppID, err) + } + } + output.Printf("Created app %s\n", app.AppID) + if app.OAuthAuthorizeURL != "" { + output.Printf("Install it: %s\n", app.OAuthAuthorizeURL) + } + if creds != nil { + output.Printf("Credentials written to %s\n", credentialsFile) + } + return nil +} + +func newUpdateCmd() *cobra.Command { + var file string + cmd := &cobra.Command{ + Use: "update --file ", + Short: "Replace an app's manifest", + Long: `Replace an app's manifest with a manifest file. The file replaces the +whole manifest, so start from 'slck apps get'. Run 'slck apps diff' first to +see what changes. + +When the change adds or removes scopes, Slack applies them only after the app +is reinstalled to the workspace, and the command says so.`, + Example: ` slck apps diff A0123456789 --file app-manifest.json + slck apps update A0123456789 --file app-manifest.json`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runUpdate(newSession(), args[0], file) + }, + } + cmd.Flags().StringVarP(&file, "file", "F", "", "Manifest file (.json, .yaml or .yml)") + return cmd +} + +func runUpdate(s *session, appID, file string) error { + manifest, err := readManifest(file) + if err != nil { + return err + } + var permissionsUpdated bool + err = s.do(func(token string) (err error) { + permissionsUpdated, err = s.api.UpdateApp(token, appID, manifest) + return err + }) + if err != nil { + return wrap("update app "+appID, err) + } + output.Printf("Updated app %s from %s\n", appID, file) + if permissionsUpdated { + output.Println("Its scopes changed: reinstall the app to the workspace for them to take effect.") + } + return nil +} + +type deleteOptions struct { + force bool + stdin io.Reader + terminal bool +} + +func newDeleteCmd() *cobra.Command { + opts := &deleteOptions{} + cmd := &cobra.Command{ + Use: "delete ", + Short: "Permanently delete an app", + Long: `Permanently delete an app with apps.manifest.delete. This cannot be +undone: the app, its tokens and its installations are gone. Asks for +confirmation unless --force is given, and refuses to run without --force +when stdin is not a terminal.`, + Example: ` slck apps delete A0123456789`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + opts.stdin = os.Stdin + opts.terminal = term.IsTerminal(int(os.Stdin.Fd())) + return runDelete(newSession(), args[0], opts) + }, + } + cmd.Flags().BoolVar(&opts.force, "force", false, "Delete without asking for confirmation") + return cmd +} + +func runDelete(s *session, appID string, opts *deleteOptions) error { + if !opts.force { + if !opts.terminal { + return errors.New("delete needs confirmation: stdin is not a terminal, so pass --force to delete without asking") + } + output.Printf("Permanently delete app %s? This cannot be undone. [y/N]: ", appID) + answer := "" + scanner := bufio.NewScanner(opts.stdin) + if scanner.Scan() { + answer = strings.ToLower(strings.TrimSpace(scanner.Text())) + } + if answer != "y" && answer != "yes" { + output.Println("Cancelled.") + return nil + } + } + if err := s.do(func(token string) error { return s.api.DeleteApp(token, appID) }); err != nil { + return wrap("delete app "+appID, err) + } + output.Printf("Deleted app %s\n", appID) + return nil +} diff --git a/internal/cmd/apps/apps_test.go b/internal/cmd/apps/apps_test.go new file mode 100644 index 0000000..b36ea5d --- /dev/null +++ b/internal/cmd/apps/apps_test.go @@ -0,0 +1,324 @@ +package apps + +import ( + "bytes" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/open-cli-collective/slack-chat-api/internal/apptoken" + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/output" +) + +const ( + accessToken = "xoxe.xoxp-1-STORED-access-canary" + refreshToken = "xoxe-1-STORED-refresh-canary" +) + +var now = time.Unix(1790000000, 0) + +// call is one request the fake Slack received. +type call struct { + method string + auth string + form map[string]string +} + +// fakeSlack answers each method with the fixture named for it; a method +// with several fixtures answers them in turn. +type fakeSlack struct { + t *testing.T + mu sync.Mutex + responses map[string][]string + calls []call +} + +func newFakeSlack(t *testing.T, responses map[string][]string) (*fakeSlack, *client.AppsClient) { + f := &fakeSlack{t: t, responses: responses} + srv := httptest.NewServer(f) + t.Cleanup(srv.Close) + return f, client.NewAppsClientWithConfig(srv.URL, nil) +} + +func (f *fakeSlack) ServeHTTP(w http.ResponseWriter, r *http.Request) { + require.NoError(f.t, r.ParseForm()) + method := strings.TrimPrefix(r.URL.Path, "/") + form := map[string]string{} + for k := range r.PostForm { + form[k] = r.PostForm.Get(k) + } + f.mu.Lock() + f.calls = append(f.calls, call{method: method, auth: r.Header.Get("Authorization"), form: form}) + queue := f.responses[method] + require.NotEmpty(f.t, queue, "unexpected call to %s", method) + fixture := queue[0] + if len(queue) > 1 { + f.responses[method] = queue[1:] + } + f.mu.Unlock() + _, _ = w.Write(readFixture(f.t, fixture)) +} + +func (f *fakeSlack) methods() []string { + var out []string + for _, c := range f.calls { + out = append(out, c.method) + } + return out +} + +func readFixture(t *testing.T, name string) []byte { + t.Helper() + b, err := os.ReadFile(filepath.Join("testdata", name)) + require.NoError(t, err) + return b +} + +type memStore struct{ value string } + +func (m *memStore) Load() (string, error) { return m.value, nil } +func (m *memStore) Save(v string) error { m.value = v; return nil } + +func storedPair(t *testing.T, exp time.Time) *memStore { + t.Helper() + v, err := apptoken.Pair{Token: accessToken, RefreshToken: refreshToken, Exp: exp.Unix()}.Encode() + require.NoError(t, err) + return &memStore{value: v} +} + +func newTestSession(api *client.AppsClient, store tokenStore) *session { + return &session{api: api, store: store, now: func() time.Time { return now }} +} + +func captureOutput(t *testing.T) *bytes.Buffer { + t.Helper() + var buf bytes.Buffer + orig := output.Writer + output.Writer = &buf + t.Cleanup(func() { output.Writer = orig }) + return &buf +} + +func TestGetPrintsTheManifestIndentedAndSorted(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{"apps.manifest.export": {"manifest-export.json"}}) + out := captureOutput(t) + + require.NoError(t, runGet(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", "")) + + assert.Equal(t, string(readFixture(t, "manifest.json")), out.String()) + require.Len(t, fake.calls, 1) + assert.Equal(t, "Bearer "+accessToken, fake.calls[0].auth) + assert.Equal(t, "A0TESTAPP01", fake.calls[0].form["app_id"]) +} + +func TestGetWritesAFile(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{"apps.manifest.export": {"manifest-export.json"}}) + captureOutput(t) + path := filepath.Join(t.TempDir(), "app.json") + + require.NoError(t, runGet(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", path)) + + got, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, string(readFixture(t, "manifest.json")), string(got)) +} + +func TestSessionRotatesANearlyExpiredTokenBeforeTheCall(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{ + "tooling.tokens.rotate": {"rotate-response.json"}, + "apps.manifest.export": {"manifest-export.json"}, + }) + captureOutput(t) + store := storedPair(t, now.Add(time.Minute)) + + require.NoError(t, runGet(newTestSession(api, store), "A0TESTAPP01", "")) + + assert.Equal(t, []string{"tooling.tokens.rotate", "apps.manifest.export"}, fake.methods()) + assert.Empty(t, fake.calls[0].auth, "rotate takes no access token") + assert.Equal(t, refreshToken, fake.calls[0].form["refresh_token"]) + assert.Equal(t, "Bearer xoxe.xoxp-1-ROTATED-access-canary", fake.calls[1].auth) + + saved, err := apptoken.Parse(store.value) + require.NoError(t, err) + assert.Equal(t, "xoxe-1-ROTATED-refresh-canary", saved.RefreshToken) + assert.Equal(t, int64(1790043200), saved.Exp) + assert.Equal(t, "T0TEST", saved.TeamID) +} + +func TestSessionRotatesOnceWhenSlackRejectsTheToken(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{ + "apps.manifest.export": {"token-expired-response.json", "manifest-export.json"}, + "tooling.tokens.rotate": {"rotate-response.json"}, + }) + captureOutput(t) + + require.NoError(t, runGet(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", "")) + + assert.Equal(t, []string{"apps.manifest.export", "tooling.tokens.rotate", "apps.manifest.export"}, fake.methods()) +} + +func TestSessionDoesNotRotateTwice(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{ + "tooling.tokens.rotate": {"rotate-response.json"}, + "apps.manifest.export": {"token-expired-response.json"}, + }) + captureOutput(t) + + err := runGet(newTestSession(api, storedPair(t, now.Add(time.Minute))), "A0TESTAPP01", "") + + require.Error(t, err) + assert.Contains(t, err.Error(), "token_expired") + assert.Contains(t, err.Error(), "set-credential --key app_config_token") + assert.Equal(t, []string{"tooling.tokens.rotate", "apps.manifest.export"}, fake.methods()) +} + +func TestValidateListsManifestProblems(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{"apps.manifest.validate": {"invalid-manifest-response.json"}}) + captureOutput(t) + + err := runValidate(newTestSession(api, storedPair(t, now.Add(time.Hour))), "testdata/manifest.yaml", "A0TESTAPP01") + + require.Error(t, err) + assert.Contains(t, err.Error(), "/settings/event_subscriptions/request_url: Event subscription requests must be sent to an https URL.") + var sent map[string]any + require.NoError(t, json.Unmarshal([]byte(fake.calls[0].form["manifest"]), &sent)) + var want map[string]any + require.NoError(t, json.Unmarshal(readFixture(t, "manifest.json"), &want)) + assert.Equal(t, want, sent, "the YAML file is sent as the same manifest as its JSON twin") + assert.Equal(t, "A0TESTAPP01", fake.calls[0].form["app_id"]) +} + +func TestDiffExitsZeroWhenTheyMatch(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{"apps.manifest.export": {"manifest-export.json"}}) + out := captureOutput(t) + + require.NoError(t, runDiff(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", "testdata/manifest.yaml")) + assert.Contains(t, out.String(), "No differences") +} + +func TestDiffExitsOneAndListsDifferences(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{"apps.manifest.export": {"manifest-export.json"}}) + out := captureOutput(t) + + err := runDiff(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", "testdata/manifest-more-scopes.json") + + var exit *exitError + require.True(t, errors.As(err, &exit)) + assert.Equal(t, 1, exit.ExitCode()) + assert.True(t, exit.Silent()) + assert.Equal(t, `~ /oauth_config/scopes/bot: ["chat:write","channels:read"] -> ["chat:write","channels:read","channels:history"] ++ /settings/interactivity: {"is_enabled":true} (file only) +`, out.String()) +} + +func TestDiffExitsTwoWhenItCannotCompare(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{}) + captureOutput(t) + + err := runDiff(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", "testdata/missing.json") + + var exit *exitError + require.True(t, errors.As(err, &exit)) + assert.Equal(t, 2, exit.ExitCode()) + assert.False(t, exit.Silent()) +} + +func TestCreateWritesCredentialsOnlyToTheFile(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{"apps.manifest.create": {"create-response.json"}}) + out := captureOutput(t) + path := filepath.Join(t.TempDir(), "creds.json") + + require.NoError(t, runCreate(newTestSession(api, storedPair(t, now.Add(time.Hour))), "testdata/manifest.json", path)) + + assert.NotContains(t, out.String(), "CANARY") + assert.Contains(t, out.String(), "Created app A0TESTAPP01") + assert.Contains(t, out.String(), "https://slack.com/oauth/v2/authorize?client_id=1111.2222") + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + var creds client.AppCredentials + b, err := os.ReadFile(path) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(b, &creds)) + assert.Equal(t, "CANARY-signing-secret-77aa", creds.SigningSecret) +} + +func TestCreateRefusesAnExistingCredentialsFileBeforeCreating(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{}) + captureOutput(t) + path := filepath.Join(t.TempDir(), "creds.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + + err := runCreate(newTestSession(api, storedPair(t, now.Add(time.Hour))), "testdata/manifest.json", path) + + require.Error(t, err) + assert.Empty(t, fake.calls, "no app is created when its credentials could not be kept") +} + +func TestCreateWithoutACredentialsFilePrintsNoSecrets(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{"apps.manifest.create": {"create-response.json"}}) + out := captureOutput(t) + + require.NoError(t, runCreate(newTestSession(api, storedPair(t, now.Add(time.Hour))), "testdata/manifest.json", "")) + assert.NotContains(t, out.String(), "CANARY") +} + +func TestUpdateSaysWhenAReinstallIsNeeded(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{"apps.manifest.update": {"update-response.json"}}) + out := captureOutput(t) + + require.NoError(t, runUpdate(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", "testdata/manifest-more-scopes.json")) + + assert.Contains(t, out.String(), "reinstall the app") + assert.Equal(t, "A0TESTAPP01", fake.calls[0].form["app_id"]) +} + +func TestDeleteRefusesWithoutATerminalOrForce(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{}) + captureOutput(t) + + err := runDelete(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", &deleteOptions{stdin: strings.NewReader("y\n")}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "--force") + assert.Empty(t, fake.calls) +} + +func TestDeleteAsksAndRespectsNo(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{}) + out := captureOutput(t) + + require.NoError(t, runDelete(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", + &deleteOptions{stdin: strings.NewReader("n\n"), terminal: true})) + + assert.Contains(t, out.String(), "Cancelled.") + assert.Empty(t, fake.calls) +} + +func TestDeleteWithConfirmationOrForce(t *testing.T) { + for name, opts := range map[string]*deleteOptions{ + "confirmed": {stdin: strings.NewReader("yes\n"), terminal: true}, + "force": {force: true}, + } { + t.Run(name, func(t *testing.T) { + fake, api := newFakeSlack(t, map[string][]string{"apps.manifest.delete": {"ok-response.json"}}) + out := captureOutput(t) + + require.NoError(t, runDelete(newTestSession(api, storedPair(t, now.Add(time.Hour))), "A0TESTAPP01", opts)) + + assert.Contains(t, out.String(), "Deleted app A0TESTAPP01") + assert.Equal(t, "A0TESTAPP01", fake.calls[0].form["app_id"]) + }) + } +} diff --git a/internal/cmd/apps/diff.go b/internal/cmd/apps/diff.go new file mode 100644 index 0000000..3efba6b --- /dev/null +++ b/internal/cmd/apps/diff.go @@ -0,0 +1,93 @@ +package apps + +import ( + "encoding/json" + "fmt" + "reflect" + "sort" + "strings" +) + +// change is one difference between a live manifest and a file, at a JSON +// pointer. +type change struct { + Pointer string + Live any + File any + // Side is "live" or "file" when the key exists on only that side. + Side string +} + +// diffManifests lists the differences between two manifests, sorted by +// pointer. Objects are compared key by key; arrays and scalars are compared +// as whole values, so a reordered scope list counts as a change. +func diffManifests(live, file json.RawMessage) ([]change, error) { + var l, f any + if err := json.Unmarshal(live, &l); err != nil { + return nil, fmt.Errorf("decode live manifest: %w", err) + } + if err := json.Unmarshal(file, &f); err != nil { + return nil, fmt.Errorf("decode manifest file: %w", err) + } + var changes []change + walk("", l, f, &changes) + sort.Slice(changes, func(i, j int) bool { return changes[i].Pointer < changes[j].Pointer }) + return changes, nil +} + +func walk(ptr string, live, file any, out *[]change) { + lm, lok := live.(map[string]any) + fm, fok := file.(map[string]any) + if !lok || !fok { + if !reflect.DeepEqual(live, file) { + *out = append(*out, change{Pointer: ptr, Live: live, File: file}) + } + return + } + keys := map[string]bool{} + for k := range lm { + keys[k] = true + } + for k := range fm { + keys[k] = true + } + for k := range keys { + child := ptr + "/" + escapePointer(k) + lv, inLive := lm[k] + fv, inFile := fm[k] + switch { + case !inFile: + *out = append(*out, change{Pointer: child, Live: lv, Side: "live"}) + case !inLive: + *out = append(*out, change{Pointer: child, File: fv, Side: "file"}) + default: + walk(child, lv, fv, out) + } + } +} + +// escapePointer escapes a key for a JSON pointer (RFC 6901). +func escapePointer(k string) string { + return strings.ReplaceAll(strings.ReplaceAll(k, "~", "~0"), "/", "~1") +} + +// describe renders a change as one line: "~" changed, "-" only in the live +// app, "+" only in the file. +func (c change) describe() string { + switch c.Side { + case "live": + return fmt.Sprintf("- %s: %s (live only)", c.Pointer, compact(c.Live)) + case "file": + return fmt.Sprintf("+ %s: %s (file only)", c.Pointer, compact(c.File)) + default: + return fmt.Sprintf("~ %s: %s -> %s", c.Pointer, compact(c.Live), compact(c.File)) + } +} + +func compact(v any) string { + b, err := json.Marshal(v) + if err != nil { + return fmt.Sprint(v) + } + return string(b) +} diff --git a/internal/cmd/apps/manifest.go b/internal/cmd/apps/manifest.go new file mode 100644 index 0000000..14f6e5e --- /dev/null +++ b/internal/cmd/apps/manifest.go @@ -0,0 +1,57 @@ +package apps + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" +) + +// readManifest reads a manifest file as JSON, or as YAML when its extension +// is .yaml or .yml, and returns it as compact JSON for the API. +func readManifest(path string) (json.RawMessage, error) { + if path == "" { + return nil, fmt.Errorf("--file is required") + } + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read manifest: %w", err) + } + var doc any + switch strings.ToLower(filepath.Ext(path)) { + case ".yaml", ".yml": + if err := yaml.Unmarshal(data, &doc); err != nil { + return nil, fmt.Errorf("parse manifest %s as YAML: %w", path, err) + } + default: + if err := json.Unmarshal(data, &doc); err != nil { + return nil, fmt.Errorf("parse manifest %s as JSON: %w", path, err) + } + } + if _, ok := doc.(map[string]any); !ok { + return nil, fmt.Errorf("manifest %s must be an object", path) + } + return json.Marshal(doc) +} + +// indent renders a manifest for people and for files: two-space indented +// JSON with sorted keys and a trailing newline, so an exported manifest +// diffs cleanly in version control. +func indent(manifest json.RawMessage) ([]byte, error) { + var doc any + if err := json.Unmarshal(manifest, &doc); err != nil { + return nil, fmt.Errorf("decode manifest: %w", err) + } + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + enc.SetIndent("", " ") + if err := enc.Encode(doc); err != nil { + return nil, err + } + return buf.Bytes(), nil +} diff --git a/internal/cmd/apps/session.go b/internal/cmd/apps/session.go new file mode 100644 index 0000000..110fc45 --- /dev/null +++ b/internal/cmd/apps/session.go @@ -0,0 +1,95 @@ +package apps + +import ( + "errors" + "fmt" + "time" + + "github.com/open-cli-collective/slack-chat-api/internal/apptoken" + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/keychain" +) + +// tokenStore loads and saves the app configuration token pair's stored JSON. +type tokenStore interface { + Load() (string, error) + Save(value string) error +} + +// session runs app manifest calls with a current access token, rotating the +// pair before a call when it is near expiry and once more when Slack rejects +// the access token. Every rotation is saved before the token is used, since +// Slack may invalidate the previous refresh token. +type session struct { + api *client.AppsClient + store tokenStore + now func() time.Time +} + +func (s *session) do(call func(token string) error) error { + raw, err := s.store.Load() + if err != nil { + return err + } + pair, err := apptoken.Parse(raw) + if err != nil { + return fmt.Errorf("stored %w; store a new pair with `slck set-credential --key app_config_token --stdin`", err) + } + rotated := false + if pair.NeedsRotation(s.now()) { + if pair, err = s.rotate(pair); err != nil { + return err + } + rotated = true + } + err = call(pair.Token) + var apiErr *client.APIError + if rotated || !errors.As(err, &apiErr) || !apiErr.ExpiredToken() { + return err + } + if pair, err = s.rotate(pair); err != nil { + return err + } + return call(pair.Token) +} + +func (s *session) rotate(pair apptoken.Pair) (apptoken.Pair, error) { + r, err := s.api.RotateToken(pair.RefreshToken) + if err != nil { + return pair, fmt.Errorf("rotate app configuration token: %w; generate a new pair at api.slack.com/apps if the refresh token was revoked", err) + } + next := apptoken.Pair{Token: r.Token, RefreshToken: r.RefreshToken, Exp: r.Exp, TeamID: r.TeamID, UserID: r.UserID} + value, err := next.Encode() + if err != nil { + return pair, err + } + if err := s.store.Save(value); err != nil { + return pair, fmt.Errorf("save rotated app configuration token: %w", err) + } + return next, nil +} + +// keyringStore is the tokenStore backed by slck's keyring bundle. +type keyringStore struct{} + +func (keyringStore) Load() (string, error) { + st, err := keychain.Open() + if err != nil { + return "", err + } + defer func() { _ = st.Close() }() + return st.AppConfigToken() +} + +func (keyringStore) Save(value string) error { + st, err := keychain.Open() + if err != nil { + return err + } + defer func() { _ = st.Close() }() + return st.SetAppConfigToken(value) +} + +func newSession() *session { + return &session{api: client.NewAppsClient(), store: keyringStore{}, now: time.Now} +} diff --git a/internal/cmd/apps/testdata/create-response.json b/internal/cmd/apps/testdata/create-response.json new file mode 100644 index 0000000..eac8ef9 --- /dev/null +++ b/internal/cmd/apps/testdata/create-response.json @@ -0,0 +1 @@ +{"ok":true,"app_id":"A0TESTAPP01","credentials":{"client_id":"1111.2222","client_secret":"CANARY-client-secret-5d1e","verification_token":"CANARY-verification-9b2c","signing_secret":"CANARY-signing-secret-77aa"},"oauth_authorize_url":"https://slack.com/oauth/v2/authorize?client_id=1111.2222&scope=chat:write"} diff --git a/internal/cmd/apps/testdata/invalid-manifest-response.json b/internal/cmd/apps/testdata/invalid-manifest-response.json new file mode 100644 index 0000000..823d7ca --- /dev/null +++ b/internal/cmd/apps/testdata/invalid-manifest-response.json @@ -0,0 +1 @@ +{"ok":false,"error":"invalid_manifest","errors":[{"message":"Event subscription requests must be sent to an https URL.","pointer":"/settings/event_subscriptions/request_url"}]} diff --git a/internal/cmd/apps/testdata/manifest-export.json b/internal/cmd/apps/testdata/manifest-export.json new file mode 100644 index 0000000..20afe33 --- /dev/null +++ b/internal/cmd/apps/testdata/manifest-export.json @@ -0,0 +1 @@ +{"ok":true,"manifest":{"display_information":{"name":"Example Bot","description":"Posts build results."},"features":{"bot_user":{"display_name":"Example Bot","always_online":false}},"oauth_config":{"scopes":{"bot":["chat:write","channels:read"]}},"settings":{"org_deploy_enabled":false,"socket_mode_enabled":false,"token_rotation_enabled":false}}} diff --git a/internal/cmd/apps/testdata/manifest-more-scopes.json b/internal/cmd/apps/testdata/manifest-more-scopes.json new file mode 100644 index 0000000..9523eb1 --- /dev/null +++ b/internal/cmd/apps/testdata/manifest-more-scopes.json @@ -0,0 +1 @@ +{"display_information":{"name":"Example Bot","description":"Posts build results."},"features":{"bot_user":{"display_name":"Example Bot","always_online":false}},"oauth_config":{"scopes":{"bot":["chat:write","channels:read","channels:history"]}},"settings":{"org_deploy_enabled":false,"socket_mode_enabled":false,"token_rotation_enabled":false,"interactivity":{"is_enabled":true}}} diff --git a/internal/cmd/apps/testdata/manifest.json b/internal/cmd/apps/testdata/manifest.json new file mode 100644 index 0000000..a9534bb --- /dev/null +++ b/internal/cmd/apps/testdata/manifest.json @@ -0,0 +1,25 @@ +{ + "display_information": { + "description": "Posts build results.", + "name": "Example Bot" + }, + "features": { + "bot_user": { + "always_online": false, + "display_name": "Example Bot" + } + }, + "oauth_config": { + "scopes": { + "bot": [ + "chat:write", + "channels:read" + ] + } + }, + "settings": { + "org_deploy_enabled": false, + "socket_mode_enabled": false, + "token_rotation_enabled": false + } +} diff --git a/internal/cmd/apps/testdata/manifest.yaml b/internal/cmd/apps/testdata/manifest.yaml new file mode 100644 index 0000000..5b0f7d8 --- /dev/null +++ b/internal/cmd/apps/testdata/manifest.yaml @@ -0,0 +1,16 @@ +display_information: + name: Example Bot + description: Posts build results. +features: + bot_user: + display_name: Example Bot + always_online: false +oauth_config: + scopes: + bot: + - chat:write + - channels:read +settings: + org_deploy_enabled: false + socket_mode_enabled: false + token_rotation_enabled: false diff --git a/internal/cmd/apps/testdata/ok-response.json b/internal/cmd/apps/testdata/ok-response.json new file mode 100644 index 0000000..31bc455 --- /dev/null +++ b/internal/cmd/apps/testdata/ok-response.json @@ -0,0 +1 @@ +{"ok":true} diff --git a/internal/cmd/apps/testdata/rotate-response.json b/internal/cmd/apps/testdata/rotate-response.json new file mode 100644 index 0000000..34d67c3 --- /dev/null +++ b/internal/cmd/apps/testdata/rotate-response.json @@ -0,0 +1 @@ +{"ok":true,"token":"xoxe.xoxp-1-ROTATED-access-canary","refresh_token":"xoxe-1-ROTATED-refresh-canary","team_id":"T0TEST","user_id":"U0TEST","iat":1790000000,"exp":1790043200} diff --git a/internal/cmd/apps/testdata/token-expired-response.json b/internal/cmd/apps/testdata/token-expired-response.json new file mode 100644 index 0000000..f68b63b --- /dev/null +++ b/internal/cmd/apps/testdata/token-expired-response.json @@ -0,0 +1 @@ +{"ok":false,"error":"token_expired"} diff --git a/internal/cmd/apps/testdata/update-response.json b/internal/cmd/apps/testdata/update-response.json new file mode 100644 index 0000000..0cf1566 --- /dev/null +++ b/internal/cmd/apps/testdata/update-response.json @@ -0,0 +1 @@ +{"ok":true,"app_id":"A0TESTAPP01","permissions_updated":true} diff --git a/internal/cmd/config/show.go b/internal/cmd/config/show.go index 89439cd..6dbeb2e 100644 --- a/internal/cmd/config/show.go +++ b/internal/cmd/config/show.go @@ -1,10 +1,13 @@ package config import ( + "time" + "github.com/spf13/cobra" "github.com/open-cli-collective/cli-common/credstore" + "github.com/open-cli-collective/slack-chat-api/internal/apptoken" appconfig "github.com/open-cli-collective/slack-chat-api/internal/config" "github.com/open-cli-collective/slack-chat-api/internal/keychain" "github.com/open-cli-collective/slack-chat-api/internal/output" @@ -45,6 +48,11 @@ type showStatus struct { Workspace string `json:"workspace,omitempty"` BotToken bool `json:"bot_token_present"` UserToken bool `json:"user_token_present"` + AppConfigToken bool `json:"app_config_token_present"` + // AppConfigExpires is the access token's expiry (RFC 3339); slck rotates + // the pair when it is used near or after this time. + AppConfigExpires string `json:"app_config_token_expires,omitempty"` + AppConfigTeamID string `json:"app_config_token_team_id,omitempty"` } func runShow(opts *showOptions) error { @@ -71,6 +79,15 @@ func runShow(opts *showOptions) error { Workspace: cfg.Workspace, BotToken: st.HasBotToken(), UserToken: st.HasUserToken(), + AppConfigToken: st.HasAppConfigToken(), + } + if status.AppConfigToken { + if raw, err := st.AppConfigToken(); err == nil { + if pair, err := apptoken.Parse(raw); err == nil { + status.AppConfigExpires = pair.Expiry().UTC().Format(time.RFC3339) + status.AppConfigTeamID = pair.TeamID + } + } } if backend == credstore.BackendFile { svc, _, _ := credstore.ParseRef(st.Ref()) @@ -94,7 +111,15 @@ func runShow(opts *showOptions) error { } output.Printf("bot_token: %s\n", presence(status.BotToken)) output.Printf("user_token: %s\n", presence(status.UserToken)) - if !status.BotToken && !status.UserToken { + appConfig := presence(status.AppConfigToken) + if status.AppConfigExpires != "" { + appConfig += ", access token expires " + status.AppConfigExpires + } + if status.AppConfigTeamID != "" { + appConfig += ", team " + status.AppConfigTeamID + } + output.Printf("app_config_token: %s\n", appConfig) + if !status.BotToken && !status.UserToken && !status.AppConfigToken { output.Println() output.Println("No credentials configured. Run 'slck init' or") output.Println("'slck set-credential --key bot_token --stdin'.") diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 0eaf7cd..09d9fbe 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -1,7 +1,9 @@ package root import ( + "errors" "fmt" + "io" "os" "github.com/spf13/cobra" @@ -9,6 +11,7 @@ import ( "github.com/open-cli-collective/cli-common/credstore" "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/cmd/apps" "github.com/open-cli-collective/slack-chat-api/internal/cmd/canvas" "github.com/open-cli-collective/slack-chat-api/internal/cmd/channels" "github.com/open-cli-collective/slack-chat-api/internal/cmd/config" @@ -122,11 +125,28 @@ func Command() *cobra.Command { return rootCmd } // Execute runs the root command func Execute() { if err := rootCmd.Execute(); err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) + os.Exit(exitCode(err, os.Stderr)) } } +// exitCode prints err and returns the process exit status: 1, or the code +// an error carries for commands whose status means more than failure (slck +// apps diff exits 1 for "differs" and 2 when it could not compare). +func exitCode(err error, stderr io.Writer) int { + var coded interface { + ExitCode() int + Silent() bool + } + if errors.As(err, &coded) { + if !coded.Silent() { + _, _ = fmt.Fprintln(stderr, err) + } + return coded.ExitCode() + } + _, _ = fmt.Fprintln(stderr, err) + return 1 +} + func init() { rootCmd.PersistentFlags().StringVarP(&outputFormat, "output", "o", "text", "Output format: text or table") rootCmd.PersistentFlags().BoolVar(&output.NoColor, "no-color", false, "Disable colored output") @@ -138,6 +158,7 @@ func init() { rootCmd.SetVersionTemplate("slck " + version.Info() + "\n") // Add subcommands + rootCmd.AddCommand(apps.NewCmd()) rootCmd.AddCommand(canvas.NewCmd()) rootCmd.AddCommand(channels.NewCmd()) rootCmd.AddCommand(users.NewCmd()) diff --git a/internal/cmd/root/root_test.go b/internal/cmd/root/root_test.go index d2db9ed..b580e64 100644 --- a/internal/cmd/root/root_test.go +++ b/internal/cmd/root/root_test.go @@ -1,9 +1,14 @@ package root import ( + "bytes" + "errors" + "fmt" "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/open-cli-collective/slack-chat-api/internal/client" "github.com/open-cli-collective/slack-chat-api/internal/testutil" ) @@ -92,3 +97,26 @@ func TestRootRegistersMeNotWhoami(t *testing.T) { t.Errorf("expected exactly one `me` command, found %d", meCount) } } + +type codedError struct { + code int + silent bool +} + +func (e codedError) Error() string { return "differs" } +func (e codedError) ExitCode() int { return e.code } +func (e codedError) Silent() bool { return e.silent } + +func TestExitCode(t *testing.T) { + var stderr bytes.Buffer + assert.Equal(t, 1, exitCode(errors.New("boom"), &stderr)) + assert.Equal(t, "boom\n", stderr.String()) + + stderr.Reset() + assert.Equal(t, 1, exitCode(fmt.Errorf("wrapped: %w", codedError{code: 1, silent: true}), &stderr)) + assert.Empty(t, stderr.String()) + + stderr.Reset() + assert.Equal(t, 2, exitCode(codedError{code: 2}, &stderr)) + assert.Equal(t, "differs\n", stderr.String()) +} diff --git a/internal/cmd/setcred/setcred.go b/internal/cmd/setcred/setcred.go index 16711de..b5af1c2 100644 --- a/internal/cmd/setcred/setcred.go +++ b/internal/cmd/setcred/setcred.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" + "github.com/open-cli-collective/slack-chat-api/internal/apptoken" "github.com/open-cli-collective/slack-chat-api/internal/keychain" "github.com/open-cli-collective/slack-chat-api/internal/output" ) @@ -28,22 +29,27 @@ type options struct { func NewCmd() *cobra.Command { opts := &options{} cmd := &cobra.Command{ - Use: "set-credential --key (--stdin | --from-env NAME)", + Use: "set-credential --key (--stdin | --from-env NAME)", Short: "Store one credential in the keyring (stdin or env ingress)", Long: `Store a single secret in the OS keyring (§1.5.2). The value is read ONLY from stdin (--stdin) or a named environment variable (--from-env NAME) — never from a flag or positional argument. Only the -keys 'bot_token' and 'user_token' are accepted. +keys 'bot_token', 'user_token' and 'app_config_token' are accepted. + +app_config_token is the app configuration token pair the 'slck apps' +commands use, as the JSON object +{"token": "xoxe.xoxp-...", "refresh_token": "xoxe-...", "exp": }. op read 'op://Vault/Slack/bot_token' | slck set-credential --key bot_token --stdin - slck set-credential --key user_token --from-env SLACK_USER_TOKEN`, + slck set-credential --key user_token --from-env SLACK_USER_TOKEN + | slck set-credential --key app_config_token --stdin`, RunE: func(cmd *cobra.Command, args []string) error { return run(opts) }, } cmd.Flags().StringVar(&opts.ref, "ref", "", "Credential ref (default: config.yml credential_ref)") - cmd.Flags().StringVar(&opts.key, "key", "", "Key to set: bot_token or user_token") + cmd.Flags().StringVar(&opts.key, "key", "", "Key to set: bot_token, user_token or app_config_token") cmd.Flags().BoolVar(&opts.stdin, "stdin", false, "Read the secret value from stdin") cmd.Flags().StringVar(&opts.fromEnv, "from-env", "", "Read the secret value from this env var") return cmd @@ -51,7 +57,7 @@ keys 'bot_token' and 'user_token' are accepted. func run(opts *options) error { if opts.key == "" { - return fmt.Errorf("--key is required (bot_token or user_token)") + return fmt.Errorf("--key is required (bot_token, user_token or app_config_token)") } if opts.stdin == (opts.fromEnv != "") { return fmt.Errorf("exactly one of --stdin or --from-env is required") @@ -76,9 +82,18 @@ func run(opts *options) error { err = st.SetBotToken(value) case keychain.KeyUserToken: err = st.SetUserToken(value) + case keychain.KeyAppConfigToken: + var pair apptoken.Pair + if pair, err = apptoken.Parse(value); err != nil { + return err + } + if value, err = pair.Encode(); err != nil { + return err + } + err = st.SetAppConfigToken(value) default: - return fmt.Errorf("key %q not allowed (allowed: %s, %s)", - opts.key, keychain.KeyBotToken, keychain.KeyUserToken) + return fmt.Errorf("key %q not allowed (allowed: %s, %s, %s)", + opts.key, keychain.KeyBotToken, keychain.KeyUserToken, keychain.KeyAppConfigToken) } if err != nil { return err diff --git a/internal/cmd/setcred/setcred_test.go b/internal/cmd/setcred/setcred_test.go index 36bd806..eb36dc3 100644 --- a/internal/cmd/setcred/setcred_test.go +++ b/internal/cmd/setcred/setcred_test.go @@ -85,3 +85,36 @@ func TestSetCredential_EmptyRejected(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "empty") } + +func TestSetCredential_AppConfigToken(t *testing.T) { + testutil.Setup(t) + const access = "xoxe.xoxp-1-SETCRED-access-canary" + const refresh = "xoxe-1-SETCRED-refresh-canary" + pair := `{"token": "` + access + `", "refresh_token": "` + refresh + `", "exp": 1790043200}` + out, err := capture(t, func() error { + return run(&options{key: keychain.KeyAppConfigToken, stdin: true, in: strings.NewReader(pair + "\n")}) + }) + require.NoError(t, err) + if leak := credstore.NoLeakAssertion([]byte(out), access, refresh); leak != nil { + t.Fatalf("set-credential echoed the value: %v", leak) + } + st, err := keychain.Open() + require.NoError(t, err) + defer func() { _ = st.Close() }() + got, err := st.AppConfigToken() + require.NoError(t, err) + require.Contains(t, got, refresh) +} + +func TestSetCredential_RejectsAMalformedAppConfigToken(t *testing.T) { + testutil.Setup(t) + _, err := capture(t, func() error { + return run(&options{key: keychain.KeyAppConfigToken, stdin: true, in: strings.NewReader("xoxe.xoxp-1-bare-canary\n")}) + }) + require.Error(t, err) + require.NotContains(t, err.Error(), "canary") + st, err := keychain.Open() + require.NoError(t, err) + defer func() { _ = st.Close() }() + require.False(t, st.HasAppConfigToken()) +} diff --git a/internal/keychain/keychain.go b/internal/keychain/keychain.go index ef5343c..52e7db2 100644 --- a/internal/keychain/keychain.go +++ b/internal/keychain/keychain.go @@ -31,10 +31,14 @@ import ( const ( KeyBotToken = "bot_token" KeyUserToken = "user_token" + // KeyAppConfigToken holds the app configuration token pair (access + // token, refresh token and expiry) as one JSON value, so a rotation + // replaces the pair in a single write. See internal/apptoken. + KeyAppConfigToken = "app_config_token" ) -// allowedKeys is slck's §1.5.2 allowlist: exactly the two bundle keys. -var allowedKeys = []string{KeyBotToken, KeyUserToken} +// allowedKeys is slck's §1.5.2 allowlist. +var allowedKeys = []string{KeyBotToken, KeyUserToken, KeyAppConfigToken} // Store is an open handle to slck's credential bundle. Construct with Open, // always Close. It carries the resolved ref so callers can report it in @@ -152,6 +156,12 @@ func (s *Store) BotToken() (string, error) { return s.get(KeyBotToken, ErrMissin // UserToken returns the user token from the keyring. func (s *Store) UserToken() (string, error) { return s.get(KeyUserToken, ErrMissingUserToken) } +// AppConfigToken returns the stored app configuration token pair as its +// JSON value. +func (s *Store) AppConfigToken() (string, error) { + return s.get(KeyAppConfigToken, ErrMissingAppConfigToken) +} + func (s *Store) get(key string, missing error) (string, error) { v, err := s.cs.Get(s.profile, key) if errors.Is(err, credstore.ErrNotFound) || (err == nil && v == "") { @@ -169,6 +179,10 @@ func (s *Store) get(key string, missing error) (string, error) { func (s *Store) SetBotToken(v string) error { return s.set(KeyBotToken, v) } func (s *Store) SetUserToken(v string) error { return s.set(KeyUserToken, v) } +// SetAppConfigToken stores the app configuration token pair. It is written +// by set-credential and again after every rotation. +func (s *Store) SetAppConfigToken(v string) error { return s.set(KeyAppConfigToken, v) } + func (s *Store) set(key, v string) error { if err := s.cs.Set(s.profile, key, v, credstore.WithOverwrite()); err != nil { return fmt.Errorf("store %s at %s: %w", key, s.ref, err) @@ -199,6 +213,9 @@ func (s *Store) del(key string) error { func (s *Store) HasBotToken() bool { return s.has(KeyBotToken) } func (s *Store) HasUserToken() bool { return s.has(KeyUserToken) } +// HasAppConfigToken reports whether an app configuration token pair is stored. +func (s *Store) HasAppConfigToken() bool { return s.has(KeyAppConfigToken) } + func (s *Store) has(key string) bool { ok, err := s.cs.Exists(s.profile, key) return err == nil && ok @@ -215,6 +232,9 @@ func (s *Store) Clear() ([]string, error) { var ( ErrMissingBotToken = errors.New("slck: no bot token in keyring — run `slck init` or `slck set-credential --key bot_token --stdin`") ErrMissingUserToken = errors.New("slck: no user token in keyring — run `slck init` or `slck set-credential --key user_token --stdin`") + // ErrMissingAppConfigToken is returned by the apps commands when no + // app configuration token pair is stored. + ErrMissingAppConfigToken = errors.New("slck: no app configuration token in keyring; generate one at api.slack.com/apps and pipe {\"token\", \"refresh_token\", \"exp\"} JSON to `slck set-credential --key app_config_token --stdin`") ) // DetectTokenType returns "bot" for xoxb-*, "user" for xoxp-*, else diff --git a/internal/noleak/noleak_test.go b/internal/noleak/noleak_test.go index c0d62b8..a209aa6 100644 --- a/internal/noleak/noleak_test.go +++ b/internal/noleak/noleak_test.go @@ -186,3 +186,33 @@ func TestNoLeak_APICommandClasses(t *testing.T) { }) } } + +// TestNoLeak_AppConfigToken stores an app configuration token pair through +// set-credential and reads its status through config show; neither the +// access token nor the refresh token may appear in any output. +func TestNoLeak_AppConfigToken(t *testing.T) { + const access = "xoxe.xoxp-1-NOLEAK-access-canary-4b7e2a" + const refresh = "xoxe-1-NOLEAK-refresh-canary-9d3f61" + pairCanaries := []string{access, access[:20], refresh, refresh[:16]} + check := func(name, out string) { + t.Helper() + if leak := credstore.NoLeakAssertion([]byte(out), pairCanaries...); leak != nil { + t.Fatalf("%s leaked the app configuration token (%v).\n--- captured ---\n%s", name, leak, out) + } + } + + testutil.Setup(t) + set := setcred.NewCmd() + set.SetArgs([]string{"--key", "app_config_token", "--stdin"}) + check("set-credential app_config_token", captureAll(t, + `{"token": "`+access+`", "refresh_token": "`+refresh+`", "exp": 1790043200}`+"\n", + func() { _ = set.Execute() })) + + for _, args := range [][]string{{"show"}, {"show", "--json"}} { + c := cfgcmd.NewCmd() + c.SetArgs(args) + out := captureAll(t, "", func() { _ = c.Execute() }) + require.Contains(t, out, "2026-") + check("config "+strings.Join(args, " "), out) + } +} From 3a7b4082cee803a563337463a4d47821297ad6ed Mon Sep 17 00:00:00 2001 From: piekstra Date: Mon, 19 Oct 2026 09:32:47 -0400 Subject: [PATCH 2/2] fix(apps): exit 2 for diff invocation errors and address review A mistyped apps diff invocation exited 1, which scripts read as drift. Argument and flag errors now exit 2. The typed error is renamed AppsAPIError and documented as AppsClient's only, the exit-2 helper is cannotCompare, the JSON carve-out list in docs/development.md names apps get, and a test covers removing the credentials file when create fails. --- docs/development.md | 5 ++++- internal/client/apps.go | 16 +++++++++------- internal/cmd/apps/apps.go | 30 +++++++++++++++++++++--------- internal/cmd/apps/apps_test.go | 33 +++++++++++++++++++++++++++++++++ internal/cmd/apps/session.go | 2 +- 5 files changed, 68 insertions(+), 18 deletions(-) diff --git a/docs/development.md b/docs/development.md index 36a19af..506760a 100644 --- a/docs/development.md +++ b/docs/development.md @@ -46,7 +46,10 @@ slack-chat-api/ ## Output Contract Resource and mutation-success commands emit text or table output. JSON is -reserved for local control-plane carve-outs, currently `slck config show --json`. +reserved for local control-plane carve-outs, currently `slck config show --json`, +and for `slck apps get`, whose output is the app's manifest itself (the +document `apps validate`, `diff`, `create` and `update` read back), not a +rendering of it. The project-level JSON-vs-text contract lives in `ARCHITECTURE.md`. Source of truth: https://github.com/open-cli-collective/slack-chat-api/blob/main/ARCHITECTURE.md diff --git a/internal/client/apps.go b/internal/client/apps.go index 9a3012a..c7795b4 100644 --- a/internal/client/apps.go +++ b/internal/client/apps.go @@ -40,15 +40,17 @@ type ManifestError struct { Pointer string `json:"pointer"` } -// APIError is a Slack answer with ok=false. Errors carries the manifest -// problems Slack lists for invalid_manifest. -type APIError struct { +// AppsAPIError is a Slack answer with ok=false from an AppsClient method. +// Errors carries the manifest problems Slack lists for invalid_manifest. +// Client (the bot and user token client) does not return it; its errors are +// matched with IsSlackError. +type AppsAPIError struct { Method string Code string Errors []ManifestError } -func (e *APIError) Error() string { +func (e *AppsAPIError) Error() string { msg := fmt.Sprintf("slack API error: %s (%s)", e.Code, e.Method) for _, m := range e.Errors { msg += fmt.Sprintf("\n %s: %s", m.Pointer, m.Message) @@ -58,7 +60,7 @@ func (e *APIError) Error() string { // ExpiredToken reports whether the error means the access token no longer // works, so a rotation may recover it. -func (e *APIError) ExpiredToken() bool { +func (e *AppsAPIError) ExpiredToken() bool { return e.Code == "token_expired" || e.Code == "invalid_auth" } @@ -99,7 +101,7 @@ func (c *AppsClient) ExportManifest(token, appID string) (json.RawMessage, error } // ValidateManifest checks a manifest, against an existing app when appID is -// set. A manifest with problems returns an *APIError listing them. +// set. A manifest with problems returns an *AppsAPIError listing them. func (c *AppsClient) ValidateManifest(token string, manifest json.RawMessage, appID string) error { params := url.Values{"manifest": {string(manifest)}} if appID != "" { @@ -170,7 +172,7 @@ func (c *AppsClient) call(method, token string, params url.Values, out any) erro return fmt.Errorf("%s: unexpected response (HTTP %d)", method, resp.StatusCode) } if !status.OK { - return &APIError{Method: method, Code: status.Error, Errors: status.Errors} + return &AppsAPIError{Method: method, Code: status.Error, Errors: status.Errors} } if out == nil { return nil diff --git a/internal/cmd/apps/apps.go b/internal/cmd/apps/apps.go index 589dcbf..6d9452f 100644 --- a/internal/cmd/apps/apps.go +++ b/internal/cmd/apps/apps.go @@ -33,10 +33,18 @@ func (e *exitError) Error() string { } return e.err.Error() } -func (e *exitError) Unwrap() error { return e.err } -func (e *exitError) ExitCode() int { return e.code } -func (e *exitError) Silent() bool { return e.silent } -func usageOrFailure(err error) error { return &exitError{code: 2, err: err} } +func (e *exitError) Unwrap() error { return e.err } +func (e *exitError) ExitCode() int { return e.code } +func (e *exitError) Silent() bool { return e.silent } + +// cannotCompare is diff's exit status 2: the comparison could not be made, +// whether from a bad invocation, an unreadable file or a failed export. +func cannotCompare(err error) error { + if err == nil { + return nil + } + return &exitError{code: 2, err: err} +} // appsHints map apps.manifest.* error codes to what to do about them. var appsHints = map[string]string{ @@ -49,7 +57,7 @@ var appsHints = map[string]string{ // wrap names the operation and adds a hint for known apps.manifest.* errors. func wrap(operation string, err error) error { - var apiErr *client.APIError + var apiErr *client.AppsAPIError if errors.As(err, &apiErr) { if hint, ok := appsHints[apiErr.Code]; ok { return fmt.Errorf("%s: %w\nHint: %s", operation, err, hint) @@ -180,20 +188,24 @@ key only the file has. Paths are JSON pointers. Exit status: 0 when they match, 1 when they differ, 2 when the comparison could not be made.`, Example: ` slck apps diff A0123456789 --file app-manifest.json`, - Args: cobra.ExactArgs(1), + // A mistyped invocation must not exit 1, which means "differs". + Args: func(cmd *cobra.Command, args []string) error { + return cannotCompare(cobra.ExactArgs(1)(cmd, args)) + }, RunE: func(cmd *cobra.Command, args []string) error { quiet(cmd) return runDiff(newSession(), args[0], file) }, } cmd.Flags().StringVarP(&file, "file", "F", "", "Manifest file (.json, .yaml or .yml)") + cmd.SetFlagErrorFunc(func(_ *cobra.Command, err error) error { return cannotCompare(err) }) return cmd } func runDiff(s *session, appID, file string) error { manifest, err := readManifest(file) if err != nil { - return usageOrFailure(err) + return cannotCompare(err) } var live json.RawMessage err = s.do(func(token string) (err error) { @@ -201,11 +213,11 @@ func runDiff(s *session, appID, file string) error { return err }) if err != nil { - return usageOrFailure(wrap("export manifest of "+appID, err)) + return cannotCompare(wrap("export manifest of "+appID, err)) } changes, err := diffManifests(live, manifest) if err != nil { - return usageOrFailure(err) + return cannotCompare(err) } if len(changes) == 0 { output.Printf("No differences between %s and %s\n", appID, file) diff --git a/internal/cmd/apps/apps_test.go b/internal/cmd/apps/apps_test.go index b36ea5d..a0351c3 100644 --- a/internal/cmd/apps/apps_test.go +++ b/internal/cmd/apps/apps_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "io" "net/http" "net/http/httptest" "os" @@ -322,3 +323,35 @@ func TestDeleteWithConfirmationOrForce(t *testing.T) { }) } } + +func TestCreateRemovesTheCredentialsFileWhenCreateFails(t *testing.T) { + _, api := newFakeSlack(t, map[string][]string{"apps.manifest.create": {"invalid-manifest-response.json"}}) + captureOutput(t) + path := filepath.Join(t.TempDir(), "creds.json") + + err := runCreate(newTestSession(api, storedPair(t, now.Add(time.Hour))), "testdata/manifest.json", path) + + require.Error(t, err) + _, statErr := os.Stat(path) + assert.True(t, os.IsNotExist(statErr), "no empty credentials file is left behind") +} + +func TestDiffInvocationErrorsExitTwo(t *testing.T) { + for name, args := range map[string][]string{ + "no app id": {"diff", "--file", "testdata/manifest.json"}, + "unknown flag": {"diff", "A0TESTAPP01", "--bogus"}, + } { + t.Run(name, func(t *testing.T) { + cmd := NewCmd() + cmd.SetArgs(args) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err := cmd.Execute() + + var exit *exitError + require.True(t, errors.As(err, &exit), "got %v", err) + assert.Equal(t, 2, exit.ExitCode()) + }) + } +} diff --git a/internal/cmd/apps/session.go b/internal/cmd/apps/session.go index 110fc45..ea201e8 100644 --- a/internal/cmd/apps/session.go +++ b/internal/cmd/apps/session.go @@ -43,7 +43,7 @@ func (s *session) do(call func(token string) error) error { rotated = true } err = call(pair.Token) - var apiErr *client.APIError + var apiErr *client.AppsAPIError if rotated || !errors.As(err, &apiErr) || !apiErr.ExpiredToken() { return err }