Skip to content

Commit eca9beb

Browse files
max-sixtyclaude
andauthored
fix: warn when --config path doesn't exist, fix shell quoting in docs (#895)
* fix: warn when --config path doesn't exist, fix shell quoting in docs Bug fixes from adversarial testing: 1. --config path warning: When users pass --config with a non-existent file path, wt now warns instead of silently falling back to defaults. Note: WORKTRUNK_CONFIG_PATH env var doesn't trigger this warning since it's commonly used for test isolation with intentionally absent paths. 2. Shell quoting in docs: Template variables are automatically shell-escaped, so user-added quotes cause issues with special characters. Fixed examples in hook docs and tips-patterns that incorrectly showed quoted variables. Also adds: - Test verifying --squash is correctly ignored with --no-commit - Tests for ANSI escape sequence handling in branch names Co-Authored-By: Claude <noreply@anthropic.com> * fix: skip ANSI branch name test on Windows Git for Windows with MSYS2 bash behaves differently and may accept branch names containing control characters. The test verifies Unix git behavior, so restrict it to Unix platforms. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 44b61be commit eca9beb

14 files changed

+376
-16
lines changed

.claude-plugin/skills/worktrunk/reference/hook.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ Triggers on all switch results: creating new worktrees, switching to existing on
4848

4949
```toml
5050
[post-switch]
51-
tmux = "[ -n \"$TMUX\" ] && tmux rename-window '{{ branch | sanitize }}'"
51+
tmux = "[ -n \"$TMUX\" ] && tmux rename-window {{ branch | sanitize }}"
5252
```
5353

5454
### pre-commit
@@ -223,6 +223,8 @@ Hash any string, including concatenations:
223223
dev = "npm run dev --port {{ (repo ~ '-' ~ branch) | hash_port }}"
224224
```
225225

226+
Variables are shell-escaped automatically — quotes around `{{ ... }}` are unnecessary and can cause issues with special characters.
227+
226228
### Worktrunk functions
227229

228230
Templates also support functions for dynamic lookups:
@@ -378,9 +380,9 @@ Different actions for production vs staging:
378380

379381
```toml
380382
post-merge = """
381-
if [ "{{ target }}" = "main" ]; then
383+
if [ {{ target }} = main ]; then
382384
npm run deploy:production
383-
elif [ "{{ target }}" = "staging" ]; then
385+
elif [ {{ target }} = staging ]; then
384386
npm run deploy:staging
385387
fi
386388
"""

.claude-plugin/skills/worktrunk/reference/tips-patterns.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ Each worktree gets its own tmux session with a multi-pane layout. Sessions are n
212212
# .config/wt.toml
213213
[post-create]
214214
tmux = """
215-
S="{{ branch | sanitize }}"
216-
W="{{ worktree_path }}"
215+
S={{ branch | sanitize }}
216+
W={{ worktree_path }}
217217
tmux new-session -d -s "$S" -c "$W" -n dev
218218
219219
# Create 4-pane layout: shell | backend / claude | frontend
@@ -231,7 +231,7 @@ echo "✓ Session '$S' — attach with: tmux attach -t $S"
231231
"""
232232

233233
[pre-remove]
234-
tmux = "tmux kill-session -t '{{ branch | sanitize }}' 2>/dev/null || true"
234+
tmux = "tmux kill-session -t {{ branch | sanitize }} 2>/dev/null || true"
235235
```
236236

237237
`pre-remove` stops all services when the worktree is removed.

docs/content/hook.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ Triggers on all switch results: creating new worktrees, switching to existing on
5656

5757
```toml
5858
[post-switch]
59-
tmux = "[ -n \"$TMUX\" ] && tmux rename-window '{{ branch | sanitize }}'"
59+
tmux = "[ -n \"$TMUX\" ] && tmux rename-window {{ branch | sanitize }}"
6060
```
6161

6262
### pre-commit
@@ -231,6 +231,8 @@ Hash any string, including concatenations:
231231
dev = "npm run dev --port {{ (repo ~ '-' ~ branch) | hash_port }}"
232232
```
233233

234+
Variables are shell-escaped automatically — quotes around `{{ ... }}` are unnecessary and can cause issues with special characters.
235+
234236
### Worktrunk functions
235237

236238
Templates also support functions for dynamic lookups:
@@ -386,9 +388,9 @@ Different actions for production vs staging:
386388

387389
```toml
388390
post-merge = """
389-
if [ "{{ target }}" = "main" ]; then
391+
if [ {{ target }} = main ]; then
390392
npm run deploy:production
391-
elif [ "{{ target }}" = "staging" ]; then
393+
elif [ {{ target }} = staging ]; then
392394
npm run deploy:staging
393395
fi
394396
"""

docs/content/tips-patterns.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,8 @@ Each worktree gets its own tmux session with a multi-pane layout. Sessions are n
224224
# .config/wt.toml
225225
[post-create]
226226
tmux = """
227-
S="{{ branch | sanitize }}"
228-
W="{{ worktree_path }}"
227+
S={{ branch | sanitize }}
228+
W={{ worktree_path }}
229229
tmux new-session -d -s "$S" -c "$W" -n dev
230230
231231
# Create 4-pane layout: shell | backend / claude | frontend
@@ -243,7 +243,7 @@ echo "✓ Session '$S' — attach with: tmux attach -t $S"
243243
"""
244244

245245
[pre-remove]
246-
tmux = "tmux kill-session -t '{{ branch | sanitize }}' 2>/dev/null || true"
246+
tmux = "tmux kill-session -t {{ branch | sanitize }} 2>/dev/null || true"
247247
```
248248

249249
`pre-remove` stops all services when the worktree is removed.

src/cli/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,7 +1086,7 @@ Triggers on all switch results: creating new worktrees, switching to existing on
10861086
10871087
```toml
10881088
[post-switch]
1089-
tmux = "[ -n \"$TMUX\" ] && tmux rename-window '{{ branch | sanitize }}'"
1089+
tmux = "[ -n \"$TMUX\" ] && tmux rename-window {{ branch | sanitize }}"
10901090
```
10911091
10921092
### pre-commit
@@ -1261,6 +1261,8 @@ Hash any string, including concatenations:
12611261
dev = "npm run dev --port {{ (repo ~ '-' ~ branch) | hash_port }}"
12621262
```
12631263
1264+
Variables are shell-escaped automatically — quotes around `{{ ... }}` are unnecessary and can cause issues with special characters.
1265+
12641266
### Worktrunk functions
12651267
12661268
Templates also support functions for dynamic lookups:
@@ -1416,9 +1418,9 @@ Different actions for production vs staging:
14161418
14171419
```toml
14181420
post-merge = """
1419-
if [ "{{ target }}" = "main" ]; then
1421+
if [ {{ target }} = main ]; then
14201422
npm run deploy:production
1421-
elif [ "{{ target }}" = "staging" ]; then
1423+
elif [ {{ target }} = staging ]; then
14221424
npm run deploy:staging
14231425
fi
14241426
"""

src/config/user/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,17 @@ impl UserConfig {
152152
}
153153

154154
builder = builder.add_source(File::from(config_path.clone()));
155+
} else if let Some(config_path) = config_path.as_ref()
156+
&& path::is_config_path_explicit()
157+
{
158+
// Warn if user explicitly specified a config path that doesn't exist
159+
crate::styling::eprintln!(
160+
"{}",
161+
crate::styling::warning_message(format!(
162+
"Config file not found: {}",
163+
config_path.display()
164+
))
165+
);
155166
}
156167

157168
// Add environment variables with WORKTRUNK prefix

src/config/user/path.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ pub fn set_config_path(path: PathBuf) {
1717
CONFIG_PATH.set(path).ok();
1818
}
1919

20+
/// Check if the config path was explicitly specified via --config CLI flag.
21+
///
22+
/// Returns true only if --config flag was used. Environment variable
23+
/// (WORKTRUNK_CONFIG_PATH) is not considered "explicit" because it's commonly
24+
/// used for test/CI isolation with intentionally non-existent paths.
25+
pub fn is_config_path_explicit() -> bool {
26+
CONFIG_PATH.get().is_some()
27+
}
28+
2029
/// Get the user config file path.
2130
///
2231
/// Priority:

tests/integration_tests/config_show.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::common::{
2-
TestRepo, repo, set_temp_home_env, setup_snapshot_settings_with_home, temp_home, wt_command,
2+
TestRepo, repo, set_temp_home_env, setup_snapshot_settings, setup_snapshot_settings_with_home,
3+
temp_home, wt_command,
34
};
45
use insta_cmd::assert_cmd_snapshot;
56
use rstest::rstest;
@@ -1750,3 +1751,20 @@ command = "llm -m gpt-4"
17501751
migration_file
17511752
);
17521753
}
1754+
1755+
/// Test that explicitly specified --config path that doesn't exist shows a warning
1756+
#[rstest]
1757+
fn test_explicit_config_path_not_found_shows_warning(repo: TestRepo) {
1758+
let settings = setup_snapshot_settings(&repo);
1759+
settings.bind(|| {
1760+
let mut cmd = wt_command();
1761+
repo.configure_wt_cmd(&mut cmd);
1762+
cmd.arg("--config")
1763+
.arg("/nonexistent/worktrunk/config.toml")
1764+
.arg("list")
1765+
.current_dir(repo.root_path());
1766+
1767+
// Should show warning about missing config file but still succeed
1768+
assert_cmd_snapshot!(cmd);
1769+
});
1770+
}

tests/integration_tests/merge.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,3 +2015,27 @@ fn test_step_rebase_accepts_tag(mut repo: TestRepo) {
20152015
Some(&feature_wt)
20162016
));
20172017
}
2018+
2019+
// =============================================================================
2020+
// Behavior verification: --squash with --no-commit
2021+
// =============================================================================
2022+
2023+
/// Verify that `--squash` is correctly ignored when `--no-commit` is passed.
2024+
///
2025+
/// This is expected behavior: squashing creates a single commit from multiple
2026+
/// commits. If `--no-commit` is passed, there's no commit to create, so squash
2027+
/// has no effect. The merge proceeds as a fast-forward to the target.
2028+
#[rstest]
2029+
fn test_merge_squash_ignored_with_no_commit(repo_with_multi_commit_feature: TestRepo) {
2030+
let repo = &repo_with_multi_commit_feature;
2031+
let feature_wt = &repo.worktrees["feature"];
2032+
2033+
// With --no-commit, squash has no effect - the merge fast-forwards
2034+
// to main without creating any new commits
2035+
assert_cmd_snapshot!(make_snapshot_cmd(
2036+
repo,
2037+
"merge",
2038+
&["main", "--squash", "--no-commit", "--no-remove"],
2039+
Some(feature_wt)
2040+
));
2041+
}

tests/integration_tests/security.rs

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,3 +432,109 @@ fn test_execute_flag_with_directive_like_branch_name(repo: TestRepo) {
432432
"Malicious code was executed alongside legitimate -x command!"
433433
);
434434
}
435+
436+
// =============================================================================
437+
// ANSI escape sequence handling in branch names
438+
// =============================================================================
439+
440+
/// Test that git rejects branch names containing ANSI escape sequences.
441+
///
442+
/// ANSI escape sequences could theoretically corrupt terminal output if they
443+
/// appeared in branch names displayed by `wt list`. However, git blocks this
444+
/// at the ref validation level: control characters (bytes < 0x20 or 0x7F)
445+
/// are rejected by git check-ref-format rule 4.
446+
///
447+
/// The escape character (`\x1b` = 27) is a control character, so git rejects it.
448+
///
449+
/// Note: Git for Windows with MSYS2 bash behaves differently and may accept
450+
/// these branch names, so this test is Unix-only.
451+
#[rstest]
452+
#[cfg(unix)]
453+
fn test_git_rejects_ansi_escape_in_branch_names(repo: TestRepo) {
454+
let shell_cmd = r#"git branch $'feature-\x1b[31mRED\x1b[0m-test'"#;
455+
456+
let output = Command::new("bash")
457+
.args(["-c", shell_cmd])
458+
.current_dir(repo.root_path())
459+
.output()
460+
.unwrap();
461+
462+
assert!(
463+
!output.status.success(),
464+
"Expected git to reject ANSI escape sequences in branch name"
465+
);
466+
467+
let stderr = String::from_utf8_lossy(&output.stderr);
468+
assert!(
469+
stderr.contains("not a valid branch name") || stderr.contains("invalid"),
470+
"Expected git to complain about invalid branch name, got: {}",
471+
stderr
472+
);
473+
}
474+
475+
/// Test that manually created refs with ANSI escapes are ignored by git.
476+
///
477+
/// Even if an attacker bypasses git's normal validation and creates a ref file
478+
/// directly in .git/refs/heads/ with ANSI codes in the filename, git ignores it.
479+
#[rstest]
480+
#[cfg(unix)]
481+
fn test_git_ignores_malformed_refs_with_ansi(repo: TestRepo) {
482+
let shell_cmd = r#"
483+
commit_sha=$(git rev-parse HEAD)
484+
printf "$commit_sha" > '.git/refs/heads/feature-'$'\x1b''[31mRED'$'\x1b''[0m-test'
485+
"#;
486+
487+
let create_result = Command::new("bash")
488+
.args(["-c", shell_cmd])
489+
.current_dir(repo.root_path())
490+
.output()
491+
.unwrap();
492+
493+
assert!(
494+
create_result.status.success(),
495+
"Failed to create malformed ref file: {}",
496+
String::from_utf8_lossy(&create_result.stderr)
497+
);
498+
499+
// Git should ignore the malformed ref
500+
let branch_output = repo.git_output(&["branch", "-a"]);
501+
assert!(
502+
!branch_output.contains("RED"),
503+
"Malformed ref with ANSI escape should not appear in branch list"
504+
);
505+
506+
// wt list should also not show it
507+
let settings = setup_snapshot_settings(&repo);
508+
settings.bind(|| {
509+
let mut cmd = wt_command();
510+
repo.configure_wt_cmd(&mut cmd);
511+
cmd.arg("list").current_dir(repo.root_path());
512+
assert_cmd_snapshot!(cmd);
513+
});
514+
}
515+
516+
/// Test that literal escape-like text in branch names displays safely.
517+
///
518+
/// Branch names like "fix-backslash-x1b-test" contain literal characters
519+
/// (not actual escape codes). Git allows this and they should display literally.
520+
#[rstest]
521+
fn test_literal_escape_like_branch_names_displayed_safely(repo: TestRepo) {
522+
let branch_name = "fix-backslash-x1b-test";
523+
524+
let result = repo.git_command().args(["branch", branch_name]).output();
525+
526+
if let Ok(output) = result
527+
&& output.status.success()
528+
{
529+
let mut settings = setup_snapshot_settings(&repo);
530+
settings.add_filter(r"\b[0-9a-f]{7,40}\b", "[SHA]");
531+
532+
settings.bind(|| {
533+
let mut cmd = wt_command();
534+
repo.configure_wt_cmd(&mut cmd);
535+
cmd.args(["list", "--branches"])
536+
.current_dir(repo.root_path());
537+
assert_cmd_snapshot!(cmd);
538+
});
539+
}
540+
}

0 commit comments

Comments
 (0)