Skip to content

Commit a63297f

Browse files
committed
fix: harden /api/set-workspace handler + traversal test (CodeRabbit on PR #16)
Two CodeRabbit follow-ups: 1. api/config_api.py:set_workspace — when get_json(silent=True) returned a non-dict (JSON array, string, number, null), the truthy fallback `or {}` was bypassed and `body.get("path", "")` raised AttributeError, which the outer Exception handler mis-reported as a 500 server error. Added isinstance(body, dict) guard that returns 400 { error: "request body must be a JSON object" } directly. Diverged from CodeRabbit's literal patch in one way: they had it raise WorkspacePathError("path is required"), but the actual problem here is a malformed body shape — the error message should match the cause so the client can fix it. 2. tests/test_workspace_path_validation.py — the traversal test escaped to /tmp itself, which is shared and could be flipped by any other test / process creating <something>/state.vscdb there. Pinned the escape target to an isolated root inside self.tmp. Also added 4 API-layer regression tests (TestSetWorkspaceApi) using Flask test_client: JSON array / string / number return 400 (not 500), plus a sanity end-to-end with a valid {path} body returning 200 and the canonical realpath. Full suite: 152/152 OK (was 148; +4 new API tests).
1 parent 75468a2 commit a63297f

2 files changed

Lines changed: 78 additions & 3 deletions

File tree

api/config_api.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,14 @@ def validate_path():
7676

7777
@bp.route("/api/set-workspace", methods=["POST"])
7878
def set_workspace():
79-
body = request.get_json(silent=True) or {}
79+
# Reject non-dict JSON bodies (array / string / number / null). Without
80+
# this, get_json returns the value directly, the truthy fallback `or {}`
81+
# is bypassed, and `body.get("path", "")` raises AttributeError — which
82+
# the outer Exception handler then mis-reports as a 500 server error
83+
# instead of a 400 client error. (CodeRabbit on PR #16.)
84+
body = request.get_json(silent=True)
85+
if not isinstance(body, dict):
86+
return jsonify({"error": "request body must be a JSON object"}), 400
8087
raw = body.get("path", "")
8188
# Validate the supplied path BEFORE storing the override (issue #15).
8289
# validate_workspace_path collapses `..` traversal AND resolves symlinks

tests/test_workspace_path_validation.py

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,16 @@ def test_rejects_directory_without_cursor_markers(self):
9999
# ─── Path-traversal class ──────────────────────────────────────
100100

101101
def test_traversal_into_non_workspace_is_rejected(self):
102-
# /tmp/<x>/real-storage/../../ → /tmp/ → no Cursor markers → reject
103-
storage = _make_cursor_workspace_dir(self.tmp)
102+
# Keep traversal target inside this test's own temp tree — escaping
103+
# to /tmp itself would be non-deterministic (any other test or
104+
# process creating a `state.vscdb` under /tmp/<dir>/state.vscdb
105+
# would flip this test's outcome).
106+
#
107+
# <self.tmp>/isolated-root/storage/../.. → <self.tmp>/isolated-root
108+
# which contains no state.vscdb under any subdir → reject on markers.
109+
isolated_root = os.path.join(self.tmp, "isolated-root")
110+
os.makedirs(isolated_root)
111+
storage = _make_cursor_workspace_dir(isolated_root)
104112
escape = os.path.join(storage, "..", "..")
105113
with self.assertRaises(WorkspacePathError):
106114
validate_workspace_path(escape)
@@ -129,5 +137,65 @@ def test_symlink_to_real_workspace_is_canonicalised_and_accepted(self):
129137
self.assertNotEqual(result, link)
130138

131139

140+
class TestSetWorkspaceApi(unittest.TestCase):
141+
"""API-layer regressions for POST /api/set-workspace.
142+
143+
The validator helper has its own coverage above; these cases exist to
144+
pin behaviour the API handler owns (request body shape handling,
145+
HTTP status mapping). Notably the non-dict-body case which used to
146+
surface as a 500 instead of a 400 — see CodeRabbit on PR #16.
147+
"""
148+
149+
def setUp(self):
150+
from flask import Flask
151+
from api.config_api import bp as config_bp
152+
153+
self.tmp = tempfile.mkdtemp(prefix="cursor-validate-api-test-")
154+
self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True)
155+
156+
app = Flask(__name__)
157+
app.config["TESTING"] = True
158+
app.register_blueprint(config_bp)
159+
self.client = app.test_client()
160+
161+
def test_non_dict_json_array_returns_400_not_500(self):
162+
# Regression: a JSON array body (truthy, non-dict) used to trip
163+
# AttributeError on body.get(...) and surface as a 500.
164+
resp = self.client.post(
165+
"/api/set-workspace",
166+
data="[]",
167+
content_type="application/json",
168+
)
169+
self.assertEqual(resp.status_code, 400)
170+
self.assertIn("error", resp.get_json())
171+
172+
def test_non_dict_json_string_returns_400(self):
173+
resp = self.client.post(
174+
"/api/set-workspace",
175+
data='"some string"',
176+
content_type="application/json",
177+
)
178+
self.assertEqual(resp.status_code, 400)
179+
180+
def test_non_dict_json_number_returns_400(self):
181+
resp = self.client.post(
182+
"/api/set-workspace",
183+
data="42",
184+
content_type="application/json",
185+
)
186+
self.assertEqual(resp.status_code, 400)
187+
188+
def test_dict_with_valid_path_returns_200_with_canonical(self):
189+
storage = _make_cursor_workspace_dir(self.tmp)
190+
resp = self.client.post(
191+
"/api/set-workspace",
192+
json={"path": storage},
193+
)
194+
self.assertEqual(resp.status_code, 200)
195+
body = resp.get_json()
196+
self.assertTrue(body["success"])
197+
self.assertEqual(body["path"], os.path.realpath(storage))
198+
199+
132200
if __name__ == "__main__":
133201
unittest.main()

0 commit comments

Comments
 (0)