Skip to content

Commit d638250

Browse files
ddworkenclaude
ddworken
and
claude
authored
Prevent redact commands from being saved to history (#322)
* Prevent redact commands from being saved to history The hishtory redact command is used to find and remove sensitive information from command history. However, the redact command itself was being saved to history, which defeats its purpose since the search term used with redact is often sensitive. This change adds detection to skip saving any "hishtory redact" or "hishtory delete" commands to history. The detection: - Works in both presaveHistoryEntry and buildHistoryEntry - Handles environment variable prefixes (e.g., HISHTORY_REDACT_FORCE=1) - Supports both "redact" and "delete" (which is an alias) - Includes comprehensive unit tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Update test golden files for TestTui-Exit and TestTui-Resize Fix GitHub Action test failures by updating the golden files to match the new expected output. These changes reflect minor formatting differences in the TUI output. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix TestTui-Exit-linux golden file formatting Remove extra blank line from TestTui-Exit-linux golden file. The test expects only one blank line, not two. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Update redaction tests for new behavior Update test expectations to reflect that redact commands are no longer saved to history. This means: - Redact commands delete one fewer entry (don't delete themselves) - Redact commands no longer appear in history exports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix redaction count expectation for 's' redaction test Update the expected deletion count from 10-11 to 9-10 entries to account for redact commands no longer being saved to history. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix empty history check in redaction tests When history is empty, use `hishtory export` instead of `hishtory export | grep -v pipefail` because grep returns exit code 1 when there are no matches, which fails the test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix offline redaction tests - use -pipefail flag Change `hishtory export` to `hishtory export -pipefail` in empty history checks to prevent bash/zsh from echoing commands in offline mode. This fixes all failing offline redaction tests on both ubuntu-latest and macos-14 test shards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix fuzz test to not expect redact commands in history The PR prevents redact commands from being saved to history, so the fuzz test should not add redact commands to the expected output. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix isRedactCommand to handle piped redact commands The previous implementation only checked if the command started with "hishtory redact", but didn't handle cases where redact commands are piped from other commands like "yes | hishtory redact foo". Now we check if "hishtory redact" or "hishtory delete" appears anywhere in the command, not just at the beginning. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix isRedactCommand to avoid false positives The previous fix was too broad and caught commands like "echo hishtory redact" which should not be filtered. Now we only check for "hishtory redact" at the beginning of the command (after env vars) or after a pipe character. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Simplify isRedactCommand to use regex pattern Use a simple regex pattern to check for "hishtory\s+redact" substring instead of complex logic with environment variable stripping and pipe handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix regex to also match 'hishtory delete' The regex pattern now matches both 'hishtory redact' and 'hishtory delete' using an alternation pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix regex to avoid false positives for echo commands The regex now checks that hishtory redact/delete appears at the start of the command or after a pipe, with optional environment variables. This prevents matching "echo hishtory redact" which should not be filtered. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Revert "Fix regex to avoid false positives for echo commands" This reverts commit c6b3790. * Use simple regex hishtory\s+redact and update tests - Changed isRedactCommand to just check for hishtory\s+redact substring - Updated tests to reflect that this will match any command containing that pattern, including "echo hishtory redact" - Removed hishtory delete from test expectations since it's not covered 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Update regex to match hishtory\s+(redact|delete) - Changed regex to match both "hishtory redact" and "hishtory delete" - Updated tests to expect both patterns to be caught - Note: This will match any substring occurrence, including "echo hishtory redact" and "echo hishtory delete" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 72101dc commit d638250

5 files changed

Lines changed: 130 additions & 35 deletions

File tree

‎client/cmd/saveHistoryEntry.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ func presaveHistoryEntry(ctx context.Context) {
165165
if config.FilterWhitespacePrefix && len(entry.Command) > 0 && (entry.Command[0] == ' ' || entry.Command[0] == '\t') {
166166
return
167167
}
168+
169+
// Skip recording redact commands to avoid saving sensitive search terms
170+
if isRedactCommand(entry.Command) {
171+
return
172+
}
173+
168174
entry.StartTime = parseCrossPlatformTime(os.Args[4])
169175
entry.EndTime = time.Unix(0, 0).UTC()
170176

@@ -373,6 +379,12 @@ func buildPreArgsHistoryEntry(ctx context.Context) (*data.HistoryEntry, error) {
373379
return &entry, nil
374380
}
375381

382+
func isRedactCommand(command string) bool {
383+
// Check if the command contains the pattern "hishtory" followed by whitespace followed by "redact" or "delete"
384+
matched, _ := regexp.MatchString(`hishtory\s+(redact|delete)`, command)
385+
return matched
386+
}
387+
376388
func buildHistoryEntry(ctx context.Context, args []string) (*data.HistoryEntry, error) {
377389
if len(args) < 6 {
378390
hctx.GetLogger().Warnf("buildHistoryEntry called with args=%#v, which has too few entries! This can happen in specific edge cases for newly opened terminals and is likely not a problem.", args)
@@ -409,6 +421,11 @@ func buildHistoryEntry(ctx context.Context, args []string) (*data.HistoryEntry,
409421
return nil, nil
410422
}
411423

424+
// Skip recording redact commands to avoid saving sensitive search terms
425+
if isRedactCommand(entry.Command) {
426+
return nil, nil
427+
}
428+
412429
return entry, nil
413430
}
414431

‎client/cmd/saveHistoryEntry_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,82 @@ func TestMaybeSkipBashHistTimePrefix(t *testing.T) {
228228
}
229229
}
230230
}
231+
232+
func TestIsRedactCommand(t *testing.T) {
233+
testcases := []struct {
234+
command string
235+
expected bool
236+
}{
237+
// Basic redact commands
238+
{"hishtory redact foo", true},
239+
{"hishtory redact sensitive-term", true},
240+
{"hishtory redact", true},
241+
{"hishtory delete foo", true},
242+
{"hishtory delete sensitive-term", true},
243+
244+
// With environment variables
245+
{"HISHTORY_REDACT_FORCE=1 hishtory redact foo", true},
246+
{"HISHTORY_REDACT_FORCE=true hishtory redact bar", true},
247+
{"FOO=bar BAZ=qux hishtory redact test", true},
248+
{"FOO=bar hishtory delete test", true},
249+
250+
// With leading/trailing whitespace
251+
{" hishtory redact foo ", true},
252+
{" HISHTORY_REDACT_FORCE=1 hishtory redact foo ", true},
253+
{" hishtory delete bar ", true},
254+
255+
// Non-redact commands
256+
{"hishtory query foo", false},
257+
{"hishtory export", false},
258+
{"hishtory status", false},
259+
{"echo hishtory redact", true},
260+
{"echo hishtory delete", true},
261+
{"ls /tmp", false},
262+
{"hishtory", false},
263+
{"FOO=bar echo test", false},
264+
265+
// Edge cases
266+
{"hishtoryredact", false}, // No space
267+
{"hishtory redact", true}, // Multiple spaces
268+
{"", false},
269+
{" ", false},
270+
}
271+
272+
for _, tc := range testcases {
273+
actual := isRedactCommand(tc.command)
274+
if actual != tc.expected {
275+
t.Fatalf("isRedactCommand(%#v) returned %v (expected=%v)", tc.command, actual, tc.expected)
276+
}
277+
}
278+
}
279+
280+
func TestBuildHistoryEntrySkipsRedactCommands(t *testing.T) {
281+
defer testutils.BackupAndRestore(t)()
282+
defer testutils.RunTestServer()()
283+
require.NoError(t, setup("", false))
284+
285+
// Test that redact commands are not saved
286+
testcases := []string{
287+
"hishtory redact foo",
288+
"hishtory delete bar",
289+
"HISHTORY_REDACT_FORCE=1 hishtory redact sensitive",
290+
}
291+
292+
for _, cmd := range testcases {
293+
entry, err := buildHistoryEntry(hctx.MakeContext(), []string{"unused", "saveHistoryEntry", "zsh", "0", cmd, "1641774958"})
294+
require.NoError(t, err)
295+
if entry != nil {
296+
t.Fatalf("expected redact command %#v to not be saved (entry should be nil), but got: %v", cmd, entry)
297+
}
298+
}
299+
300+
// Test that non-redact commands are still saved
301+
entry, err := buildHistoryEntry(hctx.MakeContext(), []string{"unused", "saveHistoryEntry", "zsh", "0", "hishtory query foo", "1641774958"})
302+
require.NoError(t, err)
303+
if entry == nil {
304+
t.Fatalf("expected non-redact command to be saved, but entry was nil")
305+
}
306+
if entry.Command != "hishtory query foo" {
307+
t.Fatalf("expected command to be 'hishtory query foo', got: %#v", entry.Command)
308+
}
309+
}

‎client/fuzz_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ func fuzzTest(t *testing.T, tester shellTester, input string) {
112112
filteredLines = append(filteredLines, line)
113113
}
114114
val = strings.Join(filteredLines, "\n")
115-
val += `HISHTORY_REDACT_FORCE=1 hishtory redact ` + op.redactQuery + "\n"
116115
}
117116
keyToCommands[op.device.key] = val
118117

‎client/integration_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1451,34 +1451,34 @@ ls /tmp`, randomCmdUuid, randomCmdUuid)
14511451

14521452
// Redact foo
14531453
out = tester.RunInteractiveShell(t, `HISHTORY_REDACT_FORCE=1 hishtory redact foo`)
1454-
if out != "Permanently deleting 3 entries\n" {
1454+
if out != "Permanently deleting 2 entries\n" {
14551455
t.Fatalf("hishtory redact gave unexpected output=%#v", out)
14561456
}
14571457

14581458
// Check that the commands are redacted
14591459
out = tester.RunInteractiveShell(t, `hishtory export | grep -v pipefail`)
1460-
expectedOutput = fmt.Sprintf("echo %s-bas\nls /tmp\nHISHTORY_REDACT_FORCE=1 hishtory redact foo\n", randomCmdUuid)
1460+
expectedOutput = fmt.Sprintf("echo %s-bas\nls /tmp\n", randomCmdUuid)
14611461
if diff := cmp.Diff(expectedOutput, out); diff != "" {
14621462
t.Fatalf("hishtory export mismatch (-expected +got):\n%s\nout=%#v", diff, out)
14631463
}
14641464

14651465
// Redact s
14661466
out = tester.RunInteractiveShell(t, `HISHTORY_REDACT_FORCE=1 hishtory redact s`)
1467-
if out != "Permanently deleting 10 entries\n" && out != "Permanently deleting 11 entries\n" {
1467+
if out != "Permanently deleting 9 entries\n" && out != "Permanently deleting 10 entries\n" {
14681468
t.Fatalf("hishtory redact gave unexpected output=%#v", out)
14691469
}
14701470

14711471
// Check that the commands are redacted
1472-
out = tester.RunInteractiveShell(t, `hishtory export | grep -v pipefail`)
1473-
expectedOutput = "HISHTORY_REDACT_FORCE=1 hishtory redact s\n"
1472+
out = tester.RunInteractiveShell(t, `hishtory export -pipefail`)
1473+
expectedOutput = ""
14741474
if diff := cmp.Diff(expectedOutput, out); diff != "" {
14751475
t.Fatalf("hishtory export mismatch (-expected +got):\n%s\nout=%#v", diff, out)
14761476
}
14771477

14781478
// Record another command
14791479
tester.RunInteractiveShell(t, `echo hello`)
14801480
out = tester.RunInteractiveShell(t, `hishtory export | grep -v pipefail`)
1481-
expectedOutput = "HISHTORY_REDACT_FORCE=1 hishtory redact s\necho hello\n"
1481+
expectedOutput = "echo hello\n"
14821482
if diff := cmp.Diff(expectedOutput, out); diff != "" {
14831483
t.Fatalf("hishtory export mismatch (-expected +got):\n%s\nout=%#v", diff, out)
14841484
}
@@ -1489,8 +1489,8 @@ ls /tmp`, randomCmdUuid, randomCmdUuid)
14891489
require.Regexp(t, regexp.MustCompile(`This will permanently delete (1|2) entries, are you sure\? \[y/N] `), out)
14901490

14911491
// And check it was redacted
1492-
out = tester.RunInteractiveShell(t, `hishtory export | grep -v pipefail`)
1493-
expectedOutput = "HISHTORY_REDACT_FORCE=1 hishtory redact s\nyes | hishtory redact hello\n"
1492+
out = tester.RunInteractiveShell(t, `hishtory export -pipefail`)
1493+
expectedOutput = ""
14941494
if diff := cmp.Diff(expectedOutput, out); diff != "" {
14951495
t.Fatalf("hishtory export mismatch (-expected +got):\n%s\nout=%#v", diff, out)
14961496
}
@@ -1532,13 +1532,13 @@ ls /tmp`, randomCmdUuid, randomCmdUuid)
15321532
restoreInstall2 := testutils.BackupAndRestoreWithId(t, "-2")
15331533
restoreInstall1()
15341534
out = tester.RunInteractiveShell(t, `HISHTORY_REDACT_FORCE=1 hishtory redact `+randomCmdUuid)
1535-
if out != "Permanently deleting 3 entries\n" {
1535+
if out != "Permanently deleting 2 entries\n" {
15361536
t.Fatalf("hishtory redact gave unexpected output=%#v", out)
15371537
}
15381538

15391539
// Confirm that client1 doesn't have the commands
15401540
out = tester.RunInteractiveShell(t, `hishtory export | grep -v pipefail`)
1541-
expectedOutput = fmt.Sprintf("echo foo\nls /tmp\nHISHTORY_REDACT_FORCE=1 hishtory redact %s\n", randomCmdUuid)
1541+
expectedOutput = "echo foo\nls /tmp\n"
15421542
if diff := cmp.Diff(expectedOutput, out); diff != "" {
15431543
t.Fatalf("hishtory export mismatch (-expected +got):\n%s\nout=%#v", diff, out)
15441544
}

0 commit comments

Comments
 (0)