Skip to content

Reject whitespace-only content in mutation commands #238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeremy merged 4 commits into from
Mar 11, 2026
Merged

Reject whitespace-only content in mutation commands #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a748846
Reject whitespace-only content in mutation commands
robzolkos Mar 10, 2026
ccc9cad
Use TrimSpace in condition only, preserve original content
robzolkos Mar 10, 2026
746d94c
Address PR review: trim values before API calls, add whitespace-only …
robzolkos Mar 11, 2026
81c545b
Remove TrimSpace reassignments, preserve original input for API calls
robzolkos Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions e2e/campfire.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ load test_helper
assert_json_value '.code' 'usage'
}

@test "campfire post with whitespace-only content shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp campfire post " "
assert_failure
assert_json_value '.error' '<message> required'
assert_json_value '.code' 'usage'
}


# Line show/delete errors

Expand Down
9 changes: 9 additions & 0 deletions e2e/cards_columns_steps.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ load test_helper
assert_json_value '.code' 'usage'
}

@test "card with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp card " "
assert_success
assert_output_contains "basecamp card"
Comment on lines +49 to +55
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This test expects whitespace-only title to return help with exit 0. If the CLI intends to reject whitespace-only required args consistently (especially in piped/CI contexts), consider expecting a usage error (non-zero + JSON envelope) similar to the “card without title” case.

Suggested change
@test "card with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp card " "
assert_success
assert_output_contains "basecamp card"
@test "card with whitespace-only title shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp card " "
assert_failure
assert_json_value '.error' '<title> required'
assert_json_value '.code' 'usage'

Copilot uses AI. Check for mistakes.
}


# Column show errors

Expand Down
58 changes: 58 additions & 0 deletions e2e/errors.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,64 @@ load test_helper
}


# Whitespace-only content errors

@test "todo with whitespace-only content shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp todo " "
assert_success
assert_output_contains "basecamp todo"
}

@test "todos create with whitespace-only content shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp todos create " "
assert_success
assert_output_contains "basecamp todos create"
}

@test "cards create with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp cards create " "
assert_success
assert_output_contains "basecamp cards create"
}

@test "messages create with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp messages create " "
assert_success
assert_output_contains "basecamp messages create"
}

@test "message with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp message " "
assert_success
assert_output_contains "basecamp message"
Comment on lines +157 to +190
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

These new tests assert that whitespace-only required args show help and exit successfully. Because help-on-success bypasses the existing missingArg() behavior (which returns a structured usage error + non-zero exit in machine output), this makes invalid input look like success to scripts and differs from the existing “missing content” tests above. Consider asserting a usage error (JSON envelope) for whitespace-only required args instead, matching how missingArg() behaves for other invalid input.

Suggested change
@test "todos create with whitespace-only content shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp todos create " "
assert_success
assert_output_contains "basecamp todos create"
}
@test "cards create with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp cards create " "
assert_success
assert_output_contains "basecamp cards create"
}
@test "messages create with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp messages create " "
assert_success
assert_output_contains "basecamp messages create"
}
@test "message with whitespace-only title shows help" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp message " "
assert_success
assert_output_contains "basecamp message"
@test "todos create with whitespace-only content shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp todos create " "
assert_failure
assert_json_value '.error' '<content> required'
assert_json_value '.code' 'usage'
}
@test "cards create with whitespace-only title shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp cards create " "
assert_failure
assert_json_value '.error' '<title> required'
assert_json_value '.code' 'usage'
}
@test "messages create with whitespace-only title shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp messages create " "
assert_failure
assert_json_value '.error' '<title> required'
assert_json_value '.code' 'usage'
}
@test "message with whitespace-only title shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'
run basecamp message " "
assert_failure
assert_json_value '.error' '<title> required'
assert_json_value '.code' 'usage'

Copilot uses AI. Check for mistakes.
}

@test "comment with whitespace-only content shows error" {
create_credentials
create_global_config '{"account_id": 99999, "project_id": 123}'

run basecamp comment 123 " "
assert_failure
assert_json_value '.error' '<content> required'
assert_json_value '.code' 'usage'
}


# Missing context errors

@test "todos without project shows help" {
Expand Down
3 changes: 2 additions & 1 deletion internal/commands/campfire.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"path/filepath"
"strconv"
"strings"

"github.com/basecamp/basecamp-sdk/go/pkg/basecamp"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -267,7 +268,7 @@ for rich text (HTML) messages.`,
}

// Show help when invoked with no message content
if messageContent == "" {
if strings.TrimSpace(messageContent) == "" {
return missingArg(cmd, "<message>")
}

Expand Down
8 changes: 7 additions & 1 deletion internal/commands/cards.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ func newCardsCreateCmd(project, cardTable *string) *cobra.Command {
}

title := args[0]
if strings.TrimSpace(title) == "" {
return cmd.Help()
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Whitespace-only titles are treated as missing input, but returning cmd.Help() bypasses missingArg() and will exit 0 in machine-output contexts. Using missingArg(cmd, "<title>") would keep interactive behavior while returning a structured usage error for scripts/--json/--agent.

Suggested change
return cmd.Help()
return missingArg(cmd, "<title>")

Copilot uses AI. Check for mistakes.
}
var content string
if len(args) > 1 {
content = args[1]
Expand Down Expand Up @@ -444,7 +447,7 @@ You can pass either a card ID or a Basecamp URL:
basecamp cards update 789 --body "new body"`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if title == "" && content == "" && due == "" && assignee == "" {
if strings.TrimSpace(title) == "" && strings.TrimSpace(content) == "" && due == "" && assignee == "" {
return noChanges(cmd)
}
Comment on lines +450 to 452
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The new no-changes guard trims whitespace, but request construction below still checks title != "" / content != "". As a result, a whitespace-only --title " " or --body " " will still be sent when combined with another field (e.g. --due ...), so whitespace-only updates are not actually rejected. Consider using strings.TrimSpace(...) when deciding whether to set req.Title/req.Content so whitespace-only values are treated as “no input.”

Copilot uses AI. Check for mistakes.

Expand Down Expand Up @@ -737,6 +740,9 @@ func NewCardCmd() *cobra.Command {
}

title := args[0]
if strings.TrimSpace(title) == "" {
return cmd.Help()
}
var content string
if len(args) > 1 {
content = args[1]
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/comment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ Comma-separated IDs add the same comment to multiple items:
}

// Show help when invoked with no content; keep error if editor was opened
if content == "" {
if strings.TrimSpace(content) == "" {
if edit {
return output.ErrUsage("Comment content required")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,7 +1176,7 @@ You can pass either an item ID or a Basecamp URL:
basecamp files update 789 --content "new content" --in my-project`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if title == "" && content == "" && itemType == "" {
if strings.TrimSpace(title) == "" && strings.TrimSpace(content) == "" && itemType == "" {
return noChanges(cmd)
}

Comment on lines +1179 to 1182
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The new no-changes guard trims whitespace, but whitespace-only values can still be sent when itemType is specified (e.g. files update 789 --type vault --title " "). If the goal is to reject whitespace-only mutations, consider treating whitespace-only title/content as “not provided” when building the per-type update requests (i.e., avoid sending raw whitespace strings).

Suggested change
if strings.TrimSpace(title) == "" && strings.TrimSpace(content) == "" && itemType == "" {
return noChanges(cmd)
}
trimmedTitle := strings.TrimSpace(title)
trimmedContent := strings.TrimSpace(content)
if trimmedTitle == "" && trimmedContent == "" && itemType == "" {
return noChanges(cmd)
}
// Use normalized values for the rest of the function so that
// whitespace-only inputs are treated as "not provided".
title = trimmedTitle
content = trimmedContent

Copilot uses AI. Check for mistakes.
Expand Down
10 changes: 9 additions & 1 deletion internal/commands/messages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"strconv"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -237,6 +238,10 @@ func newMessagesCreateCmd(project *string, messageBoard *string) *cobra.Command
body = args[1]
}

if strings.TrimSpace(title) == "" {
return cmd.Help()
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Whitespace-only titles are treated as missing input, but returning cmd.Help() directly bypasses missingArg()’s machine-output behavior (structured usage error + non-zero exit). Consider using missingArg(cmd, "<title>") here so scripts/--json/--agent runs don’t see a success exit code for invalid input.

Suggested change
return cmd.Help()
return missingArg(cmd, "<title>")

Copilot uses AI. Check for mistakes.
}

// Validate user input first, before checking account
if edit && body != "" {
return output.ErrUsage("cannot combine --edit and body argument")
Expand Down Expand Up @@ -375,7 +380,7 @@ You can pass either a message ID or a Basecamp URL:
basecamp messages update 789 --body "new body"`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if title == "" && body == "" {
if strings.TrimSpace(title) == "" && strings.TrimSpace(body) == "" {
return noChanges(cmd)
}

Comment on lines +383 to 386
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The new no-changes guard trims whitespace, but the update request still uses raw flag values. This means a whitespace-only title/body can still be sent if the other field is non-empty (e.g. --title " " --body "hi"), which contradicts “whitespace-only fields are treated as no input” and may still hit the API with invalid values. Consider building UpdateMessageRequest by only setting Subject/Content when strings.TrimSpace(...) != "" (similar to how the TUI updates only provided fields).

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -556,6 +561,9 @@ use --message-board <id> to specify which one.`,
return missingArg(cmd, "<title>")
}
title := args[0]
if strings.TrimSpace(title) == "" {
return cmd.Help()
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Same issue for the message shortcut: cmd.Help() on whitespace-only title bypasses missingArg() and will exit 0 in machine-output contexts. Use missingArg(cmd, "<title>") for consistent rejection semantics.

Suggested change
return cmd.Help()
return missingArg(cmd, "<title>")

Copilot uses AI. Check for mistakes.
}

// Body from second positional arg or --editor
var body string
Expand Down
6 changes: 6 additions & 0 deletions internal/commands/todos.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ func NewTodoCmd() *cobra.Command {
return missingArg(cmd, "<content>")
}
content := strings.Join(args, " ")
if strings.TrimSpace(content) == "" {
return cmd.Help()
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Whitespace-only input is treated as missing content, but this path returns cmd.Help() directly. That bypasses the missingArg() helper’s machine-output behavior (JSON usage error + non-zero exit) and results in exit 0/help even in non-TTY/--json/--agent contexts. Consider using missingArg(cmd, "") here so interactive users still see help, while machine consumers get a structured usage error.

Suggested change
return cmd.Help()
return missingArg(cmd, "<content>")

Copilot uses AI. Check for mistakes.
}

if err := ensureAccount(cmd, app); err != nil {
return err
Expand Down Expand Up @@ -622,6 +625,9 @@ func newTodosCreateCmd() *cobra.Command {
return missingArg(cmd, "<content>")
}
content := strings.Join(args, " ")
if strings.TrimSpace(content) == "" {
return cmd.Help()
}

if err := ensureAccount(cmd, app); err != nil {
return err
Expand Down
Loading