Issue: #1759 Branch: fix/issue-1759 · Fix commit: 30b6056ea (api/suitemove/index.php, +4/-2 code) Screen: gui/templates/testcases/suiteMove.html · BFF: api/suitemove/index.php Status: FIXED & VERIFIED (2026-09-30)

Move / Reorder Test Suites answered

403 forbidden  {"code":"forbidden","message":"Container belongs to another test project"}

whenever the container_id of an init or reorder request belonged to a different test project than the tproject_id the caller named — while the very same endpoint answers a plain 404 not_found / Container not found for a container id that does not exist anywhere.

Those two answers are the only difference a caller can observe between

• “id 56 is a test suite that lives in a project you have no rights on”, and
• “id 56 does not exist”.

So the endpoint was a cross-project existence oracle: one 403 per existing suite is enough to enumerate the suite ids of any test project of the installation, while holding rights on nothing but one's own project. Reachable on the read path (GET ?action=init) and on the write path (POST ?action=reorder).

1. api/suitemove/index.php — case 'init' and case 'reorder' — both pass the client-supplied container_id into the shared resolver suitMoveProject($db, $user, $requestedId, $containerId) (:316).
2. → suitMoveNodeInfo() (:213) loads the container with an unpredicated SELECT id, name, parent_id, node_type_id, node_order FROM nodes_hierarchy WHERE id = ? (:225-227). No project predicate, so a row of project B is fetched exactly like a row of project A.
3. :318 the real owner comes from the parent_id walk of suitMoveOwningProject() (:239-266).
4. On disagreement the resolver answered 403 forbidden — before the rights check (now :391), and before anything could be known about the caller.
5. The genuine insufficient-rights 403 is correct and untouched for a project the caller named.

The two move-path ownership checks had already been hardened to 404 by commit 13f2ca183 (“#1740: apply code-review fixes to Move/Reorder Test Suites”):

• case 'move' — moved node of a foreign project → 404 (“404, not 403: a 403 would tell a caller who has rights on project A that node N exists inside another project B.”)
• case 'move' — destination of a foreign project → 404

That commit did not touch the resolver, which came in with the endpoint itself (a45462e2c) and predates both. So the screen shipped with three of four ownership checks hardened and one left answering 403. The resolver's own docblock at :293-301 even advertised the property the code did not implement.

One call site inside suitMoveProject():

  // BEFORE
  if (intval($requestedId) > 0 && intval($requestedId) !== $owner) {
      out(array('status' => 'error', 'code' => 'forbidden',
                'message' => 'Container belongs to another test project'), 403);
  }

  // AFTER
  if (intval($requestedId) > 0 && intval($requestedId) !== $owner) {
      out(array('status' => 'error', 'code' => 'not_found',
                'message' => 'Container not found'), 404);
  }

Why the message was changed too, and not just the status code: a 403-turned-404 that still says “belongs to another test project” would leave a softer wording oracle. The message is now byte-identical to the non-existent-id answer produced a few lines above it.

• gui/templates/testcases/suiteMove.html — :263-269 already maps 404 → smv.stNotFound and :409 reserves 403 for the CSRF refusal. No new i18n key, no locale bundle touched.
• suitMoveRequireSuite(), the move-path checks and the rights 403 of a project the caller itself named.

A code review over the whole diff found the first fix incomplete: the mismatch check was guarded by intval($requestedId) > 0, so a caller that simply omitted tproject_id made the resolver retarget onto the container's own project and then answer 403 Insufficient rights… for the very id it was probing — the oracle, one step further along:

GET  ?action=init&tproject_id=52&container_id=56  -> 404 not_found   (fixed)
GET  ?action=init&container_id=56                  -> 403 forbidden    (BYPASS)
GET  ?action=init&container_id=999999              -> 404 not_found
POST ?action=reorder&container_id=56&nodelist=…    -> 403 forbidden    (BYPASS, write path)
POST ?action=move&node_id=56&position=down         -> 403 forbidden    (BYPASS, write path)

Two further findings from the same review, all closed in the same commit:

  // suitMoveProject(), 5th parameter
  function suitMoveProject(&$db, &$user, $requestedId, $containerId = 0, $leakGuard = false)
  ...
      // Naming no project at all is the same leak with one more step …
      $leakGuard = (intval($requestedId) <= 0);
  ...
      if (is_null($tproject) || !$user->hasRight($db, 'mgt_modify_tc', $tprojectId)) {
          if ($leakGuard) {
              out(array('status' => 'error', 'code' => 'not_found',
                        'message' => 'Container not found'), 404);
          }
          out(array('status' => 'error', 'code' => 'forbidden',
                    'message' => 'Insufficient rights on this test project'), 403);
      }

bash tmp/verify_1759.sh — 28/28 PASS after the code-review round (fixture tmp/fixtures_1759.php: two private projects, an editor of A only, a view-only user and a <no rights> user). Suite Regression — Issue #1759 in tmp/TLU_Test_Cases.md: 32 PASS / 0 FAIL / 1 skipped (28 API assertions + 4 screen checks).

Browser (sm1759a):

• foreign container_id → “Not found — The requested test project, container or test suite does not exist.” instead of the smv.stNotAllowed state.
• own container_id → normal screen; destination picker lists only project A's suites.
• Move down on the project root really flips nodes_hierarchy.node_order (55→0, 54→1), notice “The test suite was moved down.”
• 0 console errors/warnings; Event Viewer ERROR/WARNING count unchanged.

Screenshots: docs/screenshots/issue-1759-foreign-container-not-found.png, docs/screenshots/issue-1759-own-project-move-still-works.png.

• api/tcreorder/index.php:234 and api/tcstepsreorder/index.php:530 still answer 403 … belongs to another test project — the same defect class in the sibling BFFs.
• tlUser::hasRight() is called without its $getAccess argument at api/suitemove/index.php:347, so the private-project flag is never evaluated and a user with no project role at all passes the rights check. Separate issue.