Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions app/backend/api/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ func (s *Server) handleSessionCreate(w http.ResponseWriter, r *http.Request) {
return
}

if errMsg := validate.ValidateName(body.Name, "Session name"); errMsg != "" {
// Tightened new-name rule (no spaces) — this names a to-be-created session.
if errMsg := validate.ValidateNewName(body.Name, "Session name"); errMsg != "" {
writeError(w, http.StatusBadRequest, errMsg)
return
}
Expand Down Expand Up @@ -70,7 +71,11 @@ func (s *Server) handleSessionRename(w http.ResponseWriter, r *http.Request) {
return
}

if errMsg := validate.ValidateName(body.Name, "Session name"); errMsg != "" {
// Tightened new-name rule (no spaces) — this is the renamed-TO name. The
// URL-param `session` above stays on the permissive ValidateName so a
// pre-existing spacey session (created outside run-kit) can still be the
// rename SOURCE.
if errMsg := validate.ValidateNewName(body.Name, "Session name"); errMsg != "" {
writeError(w, http.StatusBadRequest, errMsg)
return
}
Expand Down
74 changes: 74 additions & 0 deletions app/backend/api/sessions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,80 @@ func TestSessionCreateForbiddenChars(t *testing.T) {
}
}

func TestSessionCreateSpaceRejected(t *testing.T) {
// The tightened new-name rule (validate.ValidateNewName): a to-be-created
// session name with a space is rejected — spaces bite tmux CLI targeting,
// session-group naming, and the /$server/$window routes.
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)

body := `{"name":"My problem"}`
req := httptest.NewRequest(http.MethodPost, "/api/sessions", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)

if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if ops.createSessionCalled {
t.Error("CreateSession must NOT be called for a spacey name")
}
var result map[string]string
if err := json.NewDecoder(rec.Body).Decode(&result); err != nil {
t.Fatalf("decode error: %v", err)
}
if !strings.Contains(result["error"], "cannot contain spaces") {
t.Errorf("error = %q, want containing %q", result["error"], "cannot contain spaces")
}
}

func TestSessionRenameSpaceyNewNameRejected(t *testing.T) {
// The renamed-TO name is held to the tightened rule.
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)

body := `{"name":"My problem"}`
req := httptest.NewRequest(http.MethodPost, "/api/sessions/main/rename", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)

if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if ops.renameSessionCalled {
t.Error("RenameSession must NOT be called for a spacey new name")
}
}

func TestSessionRenameSpaceyOldNameAccepted(t *testing.T) {
// The rename SOURCE (URL param) stays on the permissive ValidateName so a
// pre-existing spacey session (created outside run-kit via raw tmux) can
// still be renamed TO a safe name.
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)

body := `{"name":"My_problem"}`
req := httptest.NewRequest(http.MethodPost, "/api/sessions/My%20problem/rename", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)

if rec.Code != http.StatusOK {
t.Errorf("status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String())
}
if !ops.renameSessionCalled {
t.Fatal("RenameSession was not called")
}
if ops.renameSessionSession != "My problem" {
t.Errorf("old name = %q, want %q", ops.renameSessionSession, "My problem")
}
if ops.renameSessionName != "My_problem" {
t.Errorf("new name = %q, want %q", ops.renameSessionName, "My_problem")
}
}

func TestSessionCreateInvalidJSON(t *testing.T) {
router := newTestRouter(&mockSessionFetcher{}, &mockTmuxOps{})

Expand Down
6 changes: 4 additions & 2 deletions app/backend/api/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ func (s *Server) handleWindowCreate(w http.ResponseWriter, r *http.Request) {
// in the embedded configs). Only a non-empty name is validated. The rename
// path (handleWindowRename) still requires a non-empty, validated name.
if body.Name != "" {
if errMsg := validate.ValidateName(body.Name, "Window name"); errMsg != "" {
// Tightened new-name rule (no spaces) — this names a to-be-created window.
if errMsg := validate.ValidateNewName(body.Name, "Window name"); errMsg != "" {
writeError(w, http.StatusBadRequest, errMsg)
return
}
Expand Down Expand Up @@ -172,7 +173,8 @@ func (s *Server) handleWindowRename(w http.ResponseWriter, r *http.Request) {
return
}

if errMsg := validate.ValidateName(body.Name, "Window name"); errMsg != "" {
// Tightened new-name rule (no spaces) — this is the renamed-TO name.
if errMsg := validate.ValidateNewName(body.Name, "Window name"); errMsg != "" {
writeError(w, http.StatusBadRequest, errMsg)
return
}
Expand Down
39 changes: 39 additions & 0 deletions app/backend/api/windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,26 @@ func TestWindowCreateNonEmptyInvalidNameRejected(t *testing.T) {
}
}

func TestWindowCreateSpaceyNameRejected(t *testing.T) {
// The tightened new-name rule (validate.ValidateNewName) applies to a
// non-empty to-be-created window name.
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)

body := `{"name":"my window"}`
req := httptest.NewRequest(http.MethodPost, "/api/sessions/run-kit/windows", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)

if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if ops.createWindowCalled {
t.Error("CreateWindow must NOT be called for a spacey name")
}
}

func TestWindowKill(t *testing.T) {
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)
Expand Down Expand Up @@ -598,6 +618,25 @@ func TestWindowRenameEmptyName(t *testing.T) {
}
}

func TestWindowRenameSpaceyNameRejected(t *testing.T) {
// The renamed-TO window name is held to the tightened rule.
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)

body := `{"name":"my window"}`
req := httptest.NewRequest(http.MethodPost, "/api/windows/@1/rename", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)

if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if ops.renameWindowCalled {
t.Error("RenameWindow must NOT be called for a spacey name")
}
}

func TestWindowKeys(t *testing.T) {
ops := &mockTmuxOps{}
router := newTestRouter(&mockSessionFetcher{}, ops)
Expand Down
20 changes: 20 additions & 0 deletions app/backend/internal/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,26 @@ func ValidateName(name, label string) string {
return ""
}

// ValidateNewName validates a name for a to-be-created or renamed-to session
// or window: the permissive ValidateName rule tightened with "no spaces", so
// the safe charset the frontend's live transforms steer toward is a real
// backend contract, not just UI steering. Existing-name lookups (URL params,
// rename/kill/upload targets, session-order entries) deliberately stay on
// ValidateName — sessions created outside run-kit (raw `tmux rename-session`)
// can carry spaces and must remain operable. Hyphens stay legal here: internal
// sessions (`_rk-pin-*`, `rk-test-e2e`, group names) rely on them; the session
// hyphen→underscore rule is UI-only steering.
// Returns empty string if valid, error message if invalid.
func ValidateNewName(name, label string) string {
if msg := ValidateName(name, label); msg != "" {
return msg
}
if strings.Contains(name, " ") {
return fmt.Sprintf("%s cannot contain spaces", label)
}
return ""
}

// ValidateColorValue validates a swatch color-value descriptor: a single ANSI
// index ("4") or a two-hue blend of two indices joined by '+' ("1+3"). Every
// index must be an integer in [0, 15]. Returns empty string if valid, an error
Expand Down
56 changes: 56 additions & 0 deletions app/backend/internal/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,62 @@ func TestValidateToolName(t *testing.T) {
}
}

func TestValidateNewName(t *testing.T) {
tests := []struct {
name string
input string
label string
wantErr bool
contains string
}{
// The tightened rule: spaces rejected on NEW names.
{"space rejected", "My problem", "Session name", true, "cannot contain spaces"},
{"leading space rejected", " name", "Session name", true, "cannot contain spaces"},
{"trailing space rejected", "name ", "Session name", true, "cannot contain spaces"},
{"label in space error", "a b", "Window name", true, "Window name"},
// Inherited from the permissive ValidateName rule.
{"empty rejected", "", "Session name", true, "cannot be empty"},
{"forbidden semicolon", "my;session", "Session name", true, "forbidden characters"},
{"contains colon", "my:session", "Session name", true, "colons or periods"},
{"contains period", "my.session", "Session name", true, "colons or periods"},
// Hyphens stay legal on the backend (UI-only steering): internal
// sessions (_rk-pin-*, rk-test-e2e, group names) rely on them.
{"hyphens allowed", "my-session", "Session name", false, ""},
{"underscores allowed", "My_problem", "Session name", false, ""},
{"alphanumeric allowed", "session123", "Session name", false, ""},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := ValidateNewName(tt.input, tt.label)
if tt.wantErr && result == "" {
t.Errorf("ValidateNewName(%q) = valid, want error", tt.input)
}
if !tt.wantErr && result != "" {
t.Errorf("ValidateNewName(%q) = %q, want valid", tt.input, result)
}
if tt.contains != "" && result != "" && !contains(result, tt.contains) {
t.Errorf("ValidateNewName(%q) = %q, want error containing %q", tt.input, result, tt.contains)
}
})
}
}

// TestValidateNewNameMaxLength pins that the length bound is inherited from
// ValidateName (128), not redefined.
func TestValidateNewNameMaxLength(t *testing.T) {
long := make([]byte, 129)
for i := range long {
long[i] = 'a'
}
if result := ValidateNewName(string(long), "Session name"); result == "" {
t.Error("expected max-length error, got valid")
}
if result := ValidateNewName(string(long[:128]), "Session name"); result != "" {
t.Errorf("128-char name should be valid, got %q", result)
}
}

func TestValidateWorktreeName(t *testing.T) {
tests := []struct {
name string
Expand Down
18 changes: 12 additions & 6 deletions app/frontend/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ import { selectWindow, createSession, createWindow, splitWindow, closePane, move
import { useBoards } from "@/hooks/use-boards";
import { useWindowPins } from "@/hooks/use-window-pins";
import { usePinActions } from "@/hooks/use-pin-actions";
import { deriveNameFromPath } from "@/components/create-session-dialog";
import {
deriveNameFromPath,
finalizeSafeName,
toSafeServerName,
toSafeSessionName,
toSafeWindowName,
} from "@/lib/names";
import { useSessionContext, useUpdateNotification } from "@/contexts/session-context";
import { useOptimisticContext, useMergedSessions } from "@/contexts/optimistic-context";
import { useOptimisticAction } from "@/hooks/use-optimistic-action";
Expand Down Expand Up @@ -1423,7 +1429,7 @@ function AppShell() {


const handleCreateIframeWindow = useCallback(() => {
const name = iframeWindowName.trim();
const name = finalizeSafeName(iframeWindowName.trim());
const url = iframeWindowUrl.trim();
if (!name || !url || !sessionName) return;
createWindow(server, sessionName, name, undefined, "iframe", url)
Expand Down Expand Up @@ -1505,7 +1511,7 @@ function AppShell() {
});

const handleCreateServer = useCallback(() => {
const trimmed = createServerName.trim();
const trimmed = finalizeSafeName(createServerName.trim());
if (!trimmed || !/^[a-zA-Z0-9_-]+$/.test(trimmed)) return;
executeCreateServer(trimmed);
// Mark the just-created server pending so the route guard shows the brief
Expand Down Expand Up @@ -2754,7 +2760,7 @@ function AppShell() {
autoFocus
type="text"
value={iframeWindowName}
onChange={(e) => setIframeWindowName(e.target.value)}
onChange={(e) => setIframeWindowName(toSafeWindowName(e.target.value))}
onKeyDown={(e) => {
if (e.key === "Enter") {
e.preventDefault();
Expand Down Expand Up @@ -2793,7 +2799,7 @@ function AppShell() {
autoFocus
type="text"
value={dialogs.renameSessionName}
onChange={(e) => dialogs.setRenameSessionName(e.target.value)}
onChange={(e) => dialogs.setRenameSessionName(toSafeSessionName(e.target.value))}
onKeyDown={(e) => e.key === "Enter" && dialogs.handleRenameSession()}
onFocus={(e) => e.target.select()}
aria-label="Session name"
Expand Down Expand Up @@ -2859,7 +2865,7 @@ function AppShell() {
autoFocus
type="text"
value={createServerName}
onChange={(e) => setCreateServerName(e.target.value)}
onChange={(e) => setCreateServerName(toSafeServerName(e.target.value))}
onKeyDown={(e) => e.key === "Enter" && handleCreateServer()}
onFocus={(e) => e.target.select()}
aria-label="Server name"
Expand Down
Loading
Loading