Stage 305: PR #1750 — fix(workspace): strip surrounding quotes from Add Space path input by @nesquena-hermes

This commit is contained in:
test 2026-05-06 17:38:11 +00:00
commit 9fb2c8eee4
3 changed files with 138 additions and 1 deletions

View file

@ -1384,6 +1384,7 @@ from api.workspace import (
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
@ -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:

View file

@ -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).
@ -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():

View file

@ -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("''") == ""