Skip to content
Closed
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: 8 additions & 1 deletion api/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,7 @@ def _keep_latest_messaging_session_per_source(sessions: list[dict]) -> list[dict
resolve_trusted_workspace,
validate_workspace_to_add,
_is_blocked_system_path,
_strip_surrounding_quotes,
_workspace_blocked_roots,
)
from api.upload import handle_upload, handle_upload_extract, handle_transcribe
Expand Down Expand Up @@ -6500,7 +6501,13 @@ def _handle_file_reveal(handler, body):


def _handle_workspace_add(handler, body):
path_str = body.get("path", "").strip()
# Strip surrounding paired quotes BEFORE any further processing — macOS
# Finder's "Copy as Pathname" wraps paths in single quotes, and users
# routinely paste those quoted strings into the Add Space input.
# Doing this at the route entry means every downstream check (blocked
# system path, validate_workspace_to_add, duplicate detection) sees the
# cleaned form.
path_str = _strip_surrounding_quotes(body.get("path", "").strip())
name = body.get("name", "").strip()
auto_create = body.get("create", False)
if not path_str:
Expand Down
24 changes: 24 additions & 0 deletions api/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,25 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path:



def _strip_surrounding_quotes(path: str) -> str:
"""Strip a single pair of surrounding single or double quotes from a path string.

macOS Finder's "Copy as Pathname" (Cmd+Option+C) returns paths wrapped in
single quotes, e.g. ``'/Users/x/Documents/foo'``. Other shells and OS file
managers do similar things with double quotes. Users routinely paste these
quoted strings into the Add Space input expecting them to "just work" —
the only reason they didn't was a missing strip.

Only paired quotes are stripped (matching opener and closer). One-sided quotes
are preserved on the slim chance a path legitimately contains a literal quote
character.
"""
s = path.strip()
if len(s) >= 2 and s[0] == s[-1] and s[0] in ("'", '"'):
return s[1:-1]
return s


def validate_workspace_to_add(path: str) -> Path:
"""Validate a path for *adding* to the workspace list (less restrictive than resolve_trusted_workspace).

Expand All @@ -575,7 +594,12 @@ def validate_workspace_to_add(path: str) -> Path:

The stricter ``resolve_trusted_workspace`` is used when *using* an existing workspace
(file reads/writes) to prevent path traversal after the list is built.

Surrounding quotes (single or double) are stripped before validation —
macOS Finder's "Copy as Pathname" wraps paths in single quotes by default,
and users routinely paste those into the Add Space input.
"""
path = _strip_surrounding_quotes(path)
candidate = Path(path).expanduser().resolve()

if not candidate.exists():
Expand Down
106 changes: 106 additions & 0 deletions tests/test_workspace_add_quote_strip.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
"""Regression tests for the Add Space surrounding-quote strip.

When users use macOS Finder's "Copy as Pathname" (Cmd+Option+C) the path
arrives wrapped in single quotes by default — e.g. `'/Users/x/Documents/foo'`.
Other shells and OS file managers do similar things with double quotes.
The Add Space input would reject these as "not a directory" because the
literal quote characters became part of the path.

This file pins the behaviour:
- Surrounding paired quotes (single or double) are stripped before validation.
- Only the OUTERMOST pair is removed — internal quotes survive.
- Mismatched / unpaired quotes are preserved (path may legitimately contain one).
- Whitespace outside the quotes is also handled.
"""
import pytest

from api.workspace import _strip_surrounding_quotes


class TestStripSurroundingQuotes:
def test_unwrapped_path_unchanged(self):
assert _strip_surrounding_quotes("/Users/x/Documents/foo") == "/Users/x/Documents/foo"

def test_single_quotes_stripped(self):
# macOS Finder default
assert _strip_surrounding_quotes("'/Users/x/Documents/foo'") == "/Users/x/Documents/foo"

def test_double_quotes_stripped(self):
assert _strip_surrounding_quotes('"/Users/x/Documents/foo"') == "/Users/x/Documents/foo"

def test_outer_whitespace_stripped_first(self):
# User pastes with trailing whitespace, then the quotes are visible
assert (
_strip_surrounding_quotes(" '/Users/x/Documents/foo' ")
== "/Users/x/Documents/foo"
)

def test_only_outermost_pair_removed(self):
# Paths can legitimately contain quote characters mid-string
assert (
_strip_surrounding_quotes("'/Users/x/it's-mine/foo'")
== "/Users/x/it's-mine/foo"
)

def test_unpaired_leading_quote_preserved(self):
# Lone quote that doesn't have a partner — assume it's part of the path
assert _strip_surrounding_quotes("'/Users/x/foo") == "'/Users/x/foo"

def test_unpaired_trailing_quote_preserved(self):
assert _strip_surrounding_quotes("/Users/x/foo'") == "/Users/x/foo'"

def test_mismatched_quote_pair_preserved(self):
# ' on one side, " on the other — not a paired quote, leave alone
assert _strip_surrounding_quotes("'/Users/x/foo\"") == "'/Users/x/foo\""

def test_empty_string(self):
assert _strip_surrounding_quotes("") == ""

def test_just_a_pair_of_quotes(self):
# Edge case: someone pastes only the quotes — strip to empty
assert _strip_surrounding_quotes("''") == ""
assert _strip_surrounding_quotes('""') == ""

def test_non_quote_paired_chars_preserved(self):
# Don't strip arbitrary matching first-and-last chars
assert _strip_surrounding_quotes("/foo/") == "/foo/"
assert _strip_surrounding_quotes("aaa") == "aaa"


class TestWorkspaceAddRouteStripsQuotes:
"""End-to-end: when a quoted path is POSTed to /api/workspaces/add, the
server should accept it as if the quotes weren't there.

This is a tiny smoke test using the validate_workspace_to_add helper
directly (the route handler also calls _strip_surrounding_quotes via
the import in api/routes.py — verified by the unit tests above).
"""

def test_validate_unwraps_quoted_path_for_existing_dir(self, tmp_path):
from api.workspace import validate_workspace_to_add

d = tmp_path / "my workspace with spaces"
d.mkdir()
# Quoted form — what Finder pastes
quoted = f"'{d}'"
p = validate_workspace_to_add(quoted)
assert str(p) == str(d.resolve())

def test_validate_unwraps_double_quoted_path(self, tmp_path):
from api.workspace import validate_workspace_to_add

d = tmp_path / "my-workspace"
d.mkdir()
quoted = f'"{d}"'
p = validate_workspace_to_add(quoted)
assert str(p) == str(d.resolve())

def test_validate_quote_only_resolves_to_empty_after_strip(self):
"""`''` strips to `""`; the empty-string check belongs at the route handler
layer (which returns "path is required"), not the validator. validate_workspace_to_add
on `""` resolves to the process CWD, which may or may not be a directory —
not the validator's responsibility. This test pins that the strip happens
and the validator is then handed the empty form, not anything corrupted.
"""
# Direct strip check — confirms the layer responsible for the strip works.
assert _strip_surrounding_quotes("''") == ""
Loading