Skip to content

feat: replace static token with configurable credential command#61

Closed
cboone wants to merge 1 commit intomainfrom
claude/configurable-credential-storage-sQtTt
Closed

feat: replace static token with configurable credential command#61
cboone wants to merge 1 commit intomainfrom
claude/configurable-credential-storage-sQtTt

Conversation

@cboone
Copy link
Copy Markdown
Owner

@cboone cboone commented Feb 27, 2026

Replace --token/FM_TOKEN with --credential-command/FM_CREDENTIAL_COMMAND
that executes a user-specified shell command to retrieve the API token.
This allows integration with any secret store (macOS Keychain, libsecret,
1Password CLI, pass, etc.).

Platform defaults retrieve the token from the OS keychain automatically:

  • macOS: security find-generic-password -s fm -a fastmail -w
  • Linux: secret-tool lookup service fm

https://claude.ai/code/session_01Cim6GJ6UqAkHeetJSdRgNK

Replace --token/FM_TOKEN with --credential-command/FM_CREDENTIAL_COMMAND
that executes a user-specified shell command to retrieve the API token.
This allows integration with any secret store (macOS Keychain, libsecret,
1Password CLI, pass, etc.).

Platform defaults retrieve the token from the OS keychain automatically:
- macOS: security find-generic-password -s fm -a fastmail -w
- Linux: secret-tool lookup service fm

https://claude.ai/code/session_01Cim6GJ6UqAkHeetJSdRgNK
Copilot AI review requested due to automatic review settings February 27, 2026 03:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the static API token configuration (--token / FM_TOKEN) with a configurable credential command (--credential-command / FM_CREDENTIAL_COMMAND) that is executed to retrieve the token at runtime, with platform defaults on macOS/Linux to read from the OS credential store.

Changes:

  • Add credential-command based token resolution (with macOS/Linux defaults) and wire it into client creation.
  • Update CLI error hints and docs to reflect the new authentication mechanism.
  • Update scrut CLI integration tests and unit tests to use --credential-command / FM_CREDENTIAL_COMMAND.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
cmd/root.go Implements --credential-command binding, platform defaults, and token resolution via shell command.
cmd/archive.go Updates auth failure hint string to credential-command wording.
cmd/draft.go Updates auth failure hint string to credential-command wording.
cmd/dryrun_test.go Updates test harness to pass token via --credential-command.
cmd/flag.go Updates auth failure hint string to credential-command wording.
cmd/list.go Updates auth failure hint string to credential-command wording.
cmd/mailboxes.go Updates auth failure hint string to credential-command wording.
cmd/mark-read.go Updates auth failure hint string to credential-command wording.
cmd/move.go Updates auth failure hint string to credential-command wording.
cmd/read.go Updates auth failure hint string to credential-command wording.
cmd/search.go Updates auth failure hint string to credential-command wording.
cmd/session.go Updates auth failure hint string to credential-command wording.
cmd/sieve_activate.go Updates auth failure hint string to credential-command wording.
cmd/sieve_create.go Updates auth failure hint string to credential-command wording.
cmd/sieve_deactivate.go Updates auth failure hint string to credential-command wording.
cmd/sieve_delete.go Updates auth failure hint string to credential-command wording.
cmd/sieve_list.go Updates auth failure hint string to credential-command wording.
cmd/sieve_show.go Updates auth failure hint string to credential-command wording.
cmd/sieve_validate.go Updates auth failure hint string to credential-command wording.
cmd/spam.go Updates auth failure hint string to credential-command wording.
cmd/stats.go Updates auth failure hint string to credential-command wording.
cmd/summary.go Updates auth failure hint string to credential-command wording.
cmd/unflag.go Updates auth failure hint string to credential-command wording.
docs/CLI-REFERENCE.md Replaces global flag documentation and auth hint examples for credential-command.
docs/CLAUDE-CODE-GUIDE.md Updates setup instructions to keychain/libsecret + credential_command config.
README.md Updates runtime setup and configuration sections for credential-command + platform defaults.
Makefile Updates live test target description to reference FM_CREDENTIAL_COMMAND.
.env.example Replaces FM_TOKEN with FM_CREDENTIAL_COMMAND and documents platform defaults/examples.
tests/arguments.md Updates scrut tests to use FM_CREDENTIAL_COMMAND naming and new auth error expectations.
tests/errors.md Updates scrut error expectations/messages to credential-command based auth.
tests/flags.md Updates scrut flag/config tests to use credential-command and updated error text.
tests/live.md Updates live test preconditions and examples to credential-command.
tests/sieve.md Updates sieve scrut tests for credential-command-based auth failures.
Comments suppressed due to low confidence (2)

tests/arguments.md:131

  • This test unsets FM_CREDENTIAL_COMMAND, which causes fm to fall back to the platform default credential command. If a developer has a valid token stored in the OS keychain/secret service, the command may succeed and the test can unexpectedly hit the real API (or fail differently). For deterministic/offline behavior, explicitly set FM_CREDENTIAL_COMMAND to a known failing command in the test environment.
$ env -u FM_CREDENTIAL_COMMAND -u FM_SESSION_URL -u FM_FORMAT -u FM_ACCOUNT_ID HOME=/nonexistent $TESTDIR/../fm search --from alice@test.com 2>&1
{
  "error": "authentication_failed",
* (glob+)
[1]

docs/CLI-REFERENCE.md:16

  • The Global Flags section says config sources resolve as "flags > environment variables > config file", but cmd/root.go also applies a platform default credential command when credential_command is unset. This doc should include the platform default step (and clarify Linux uses libsecret/secret-tool rather than “OS keychain”).
| `--credential-command` | `FM_CREDENTIAL_COMMAND` | OS keychain (macOS/Linux)      | Shell command that prints the API token to stdout |
| `--session-url` | `FM_SESSION_URL` | `https://api.fastmail.com/jmap/session` | Fastmail session endpoint         |
| `--format`      | `FM_FORMAT`      | `json`                                  | Output format: `json` or `text`   |
| `--account-id`  | `FM_ACCOUNT_ID`  | (auto-detected)                         | Fastmail account ID override      |
| `--config`      | --               | `~/.config/fm/config.yaml`              | Config file path                  |
| `--version`     | --               | --                                      | Print version and exit              |

Configuration sources are resolved in priority order: flags > environment variables > config file.

Comment thread cmd/root.go
if err := cmd.Run(); err != nil {
detail := strings.TrimSpace(stderr.String())
if detail != "" {
return "", fmt.Errorf("credential command failed: %w\n%s", err, detail)
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveToken() appends stderr output to the returned error using a literal newline ("...%w\n%s"). When --format text is used, TextFormatter.FormatError prints message verbatim, so this produces multi-line error output and will break scrut expectations that assume a single Error [...] line followed immediately by the Hint: line. Consider folding stderr into the same line (e.g., ...: <err>: <detail> with whitespace/newlines collapsed) or omitting stderr from message and exposing it elsewhere.

Suggested change
return "", fmt.Errorf("credential command failed: %w\n%s", err, detail)
// Collapse all internal whitespace (including newlines) into single spaces
detail = strings.Join(strings.Fields(detail), " ")
return "", fmt.Errorf("credential command failed: %w: %s", err, detail)

Copilot uses AI. Check for mistakes.
Comment thread README.md
```bash
export FM_CREDENTIAL_COMMAND="op read op://Private/Fastmail/token"
```

Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README removed the prior security guidance about not placing secrets in committed files, but the new credential_command can still easily embed a literal token (e.g., echo <token> in config or shell history). Consider adding a short security note here warning against hardcoding tokens in config/CLI flags and recommending keychain/secret-tool/op/pass-style commands instead.

Suggested change
**Security note:** Do not hardcode API tokens directly into `FM_CREDENTIAL_COMMAND`, shell commands, or config files (for example, avoid `FM_CREDENTIAL_COMMAND='echo fmu1-...'`). Always retrieve secrets from an OS keychain or a dedicated secret manager/CLI (such as `security`, `secret-tool`, `op`, `pass`, or similar), and ensure the command prints only the token value.

Copilot uses AI. Check for mistakes.
Comment thread cmd/root.go
Comment on lines +140 to +143
var stdout, stderr bytes.Buffer
cmd := exec.Command("sh", "-c", credCmd)
cmd.Stdout = &stdout
cmd.Stderr = &stderr
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveToken() runs the credential command with no timeout/cancellation. If the secret backend blocks (e.g., keychain prompt, locked keyring), fm will hang indefinitely. Consider using exec.CommandContext with a reasonable timeout and returning a clear error when the command exceeds it.

Copilot uses AI. Check for mistakes.
Comment thread tests/errors.md
Comment on lines +8 to +12
$ env -u FM_CREDENTIAL_COMMAND -u FM_SESSION_URL -u FM_FORMAT -u FM_ACCOUNT_ID HOME=/nonexistent $TESTDIR/../fm session 2>&1
{
"error": "authentication_failed",
"message": "no token configured; set FM_TOKEN, --token, or token in config file",
"hint": "Check your token in FM_TOKEN or config file"
"message": "credential command failed: *", (glob)
"hint": "Check your credential command or the token it returns"
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These scrut tests unset FM_CREDENTIAL_COMMAND, which causes fm to fall back to the platform default credential command. On developer machines where the default keychain/secret entry exists, the command can succeed and the tests may start making real API calls (or fail with different errors), making the suite non-deterministic. To keep tests hermetic, explicitly set FM_CREDENTIAL_COMMAND to a known failing command (or an echo that returns empty) within the test env instead of relying on the absence of configuration.

Copilot uses AI. Check for mistakes.
Comment thread .env.example
@@ -1,7 +1,17 @@
# JMAP bearer token (required)
# Generate at Fastmail: Settings > Privacy & Security > Integrations > API tokens
# Shell command that prints the JMAP bearer token to stdout (required)
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says FM_CREDENTIAL_COMMAND is “required”, but the implementation uses platform defaults on macOS/Linux when it’s unset. Consider rewording to avoid implying the env var must always be set (e.g., “required on platforms without a default, or when overriding the default”).

Suggested change
# Shell command that prints the JMAP bearer token to stdout (required)
# Shell command that prints the JMAP bearer token to stdout (required on platforms without a default, or to override the default)

Copilot uses AI. Check for mistakes.
Comment thread tests/flags.md
Comment on lines +17 to +21
$ env -u FM_CREDENTIAL_COMMAND -u FM_SESSION_URL -u FM_FORMAT -u FM_ACCOUNT_ID HOME=/nonexistent $TESTDIR/../fm session 2>&1
{
"error": "authentication_failed",
"message": "no token configured; set FM_TOKEN, --token, or token in config file",
"hint": "Check your token in FM_TOKEN or config file"
"message": "credential command failed: *", (glob)
"hint": "Check your credential command or the token it returns"
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests rely on unsetting FM_CREDENTIAL_COMMAND to force an auth failure, but fm will then fall back to the platform default credential command. If a developer has already stored a token in the OS keychain/secret service, the command may succeed and the test can unexpectedly contact the real API (or fail with different errors). Consider explicitly setting FM_CREDENTIAL_COMMAND to a deterministic failing command for hermetic tests.

Copilot uses AI. Check for mistakes.
Comment thread tests/sieve.md
Comment on lines +133 to 138
$ env -u FM_CREDENTIAL_COMMAND -u FM_SESSION_URL -u FM_FORMAT -u FM_ACCOUNT_ID HOME=/nonexistent $TESTDIR/../fm sieve list 2>&1
{
"error": "authentication_failed",
"message": "no token configured; set FM_TOKEN, --token, or token in config file",
"hint": "Check your token in FM_TOKEN or config file"
"message": "credential command failed: *", (glob)
"hint": "Check your credential command or the token it returns"
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test unsets FM_CREDENTIAL_COMMAND, which triggers the platform default credential command. If a developer has already stored a token in their OS keychain/secret service, the command can succeed and the test may start making real API calls (or fail with unexpected errors). For hermetic CLI tests, explicitly set FM_CREDENTIAL_COMMAND to a deterministic failing command in the environment for these blocks.

Copilot uses AI. Check for mistakes.
@cboone cboone closed this Mar 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants