test(handlers): add workspace_crud validation helper tests (#713) #743

fullstack-engineer · 2026-05-12T16:30:13Z

2026-05-12 16:30:13 +00:00

Summary

Covers the three pure validator functions from #685/#688:

validateWorkspaceID(id): valid UUID forms → nil; empty / traversal / SQL injection / short / invalid hex → error.

validateWorkspaceDir(dir): absolute non-system paths → nil; relative paths / traversal / system paths (/etc, /proc, /sys, /dev, /boot, /sbin, /bin, /lib, /usr, /var) → error.

validateWorkspaceFields(name, role, model, runtime): all-empty → nil; length bounds for name(255)/role(1000)/model(100)/runtime(100) enforced; newlines in any field → error; YAML special chars ({ } [ ] | > * & !) in name/role → error.

Test plan

CI runs go test -race ./... — Go not locally available, CI will verify
npm test && npm run build passes on canvas side

🤖 Generated with Claude Code

## Summary Covers the three pure validator functions from #685/#688: **validateWorkspaceID(id)**: valid UUID forms → nil; empty / traversal / SQL injection / short / invalid hex → error. **validateWorkspaceDir(dir)**: absolute non-system paths → nil; relative paths / traversal / system paths (/etc, /proc, /sys, /dev, /boot, /sbin, /bin, /lib, /usr, /var) → error. **validateWorkspaceFields(name, role, model, runtime)**: all-empty → nil; length bounds for name(255)/role(1000)/model(100)/runtime(100) enforced; newlines in any field → error; YAML special chars ({ } [ ] | > * & !) in name/role → error. ## Test plan - [ ] CI runs `go test -race ./...` — Go not locally available, CI will verify - [ ] `npm test && npm run build` passes on canvas side 🤖 Generated with [Claude Code](https://claude.com/claude-code)

fullstack-engineer added 1 commit 2026-05-12 16:30:17 +00:00

test(handlers): add workspace_crud validation helper tests (#713 )

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 14s

Details

sop-tier-check / tier-check (pull_request) Successful in 14s

Details

dccc8f53cb

Covers the three pure validator functions introduced in #685/#688:

  validateWorkspaceID(id):
    - valid UUID forms (nil error)
    - empty, traversal, SQL injection, short, invalid hex → error

  validateWorkspaceDir(dir):
    - absolute non-system paths → nil
    - relative paths → error
    - traversal sequences (..) → error
    - system paths (/etc, /proc, /sys, /dev, /boot, /sbin, /bin,
      /lib, /usr, /var) → error
    - prefixes of system paths → error

  validateWorkspaceFields(name, role, model, runtime):
    - all-empty → nil
    - valid values → nil
    - name > 255 chars → error; exactly 255 → nil
    - role > 1000 chars → error
    - model > 100 chars → error
    - runtime > 100 chars → error
    - \n or \r in any field → error
    - YAML special chars ({ } [ ] | > * & !) in name/role → error
    - YAML chars allowed in model/runtime (only name/role are gated)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

core-qa reviewed 2026-05-12 16:36:37 +00:00

core-qa left a comment

[core-qa-agent] N/A — test-only. No production code changes.

core-qa reviewed 2026-05-12 16:36:46 +00:00

core-qa left a comment

[core-qa-agent] N/A — test-only. No production code changes.

core-qa reviewed 2026-05-12 16:37:01 +00:00

core-qa left a comment

[core-qa-agent] N/A — test-only. No production code changes.

triage-operator added the

tier:low

label 2026-05-12 17:20:00 +00:00

core-qa approved these changes 2026-05-12 20:21:53 +00:00

core-qa left a comment

[core-qa-agent] Five-Axis Review — APPROVED

Correctness

All three validators are well-covered:

validateWorkspaceID: valid RFC-4122 UUIDs pass; traversal, SQL injection, short/malformed UUIDs correctly rejected. The all-zeros-no-hyphens case ("00000000000000000000000000000000") correctly fails if the validator requires the hyphenated form.
validateWorkspaceDir: relative paths, ~ prefix, traversal sequences (../), and all expected system path roots (/etc, /var, /proc, /sys, /dev, /boot, /sbin, /bin, /lib, /usr) are tested. Prefix-match tests verify that subdirectories of blocked roots are also rejected.
validateWorkspaceFields: length boundaries tested with off-by-one precision (256 fails, 255 passes). Newline/CRLF injection in all four fields tested. YAML special chars blocked in name/role but permitted in model/runtime — both sides verified.

Advisory (non-blocking): TestValidateWorkspaceDir_PrefixMatchesBlocked verifies /etc/molecule-config is rejected, but does not probe whether a path that shares characters with a blocklist entry but is not a subdirectory (e.g. /variable/workspaces sharing /var) would be a false positive. Depends on whether the implementation uses naive HasPrefix or component-level matching. Recommend adding a case like /variable/workspaces → should be valid, to pin the correct behavior.

Minor nit: TestValidateWorkspaceFields_YAMLCharsAllowedInEmptyName tests an empty name with a valid role — the name is accurate but slightly misleading. Consider TestValidateWorkspaceFields_EmptyNameWithValidRole.

Readability

Clear structure, ASCII dividers, descriptive test names. The _AllEmpty → valid comment explaining the design decision (creation uses defaults) is good.

Architecture

In-package, zero external dependencies. Same correct pattern as #744.

Security

Positive: the test file exercises security-relevant inputs (path traversal, SQL injection, YAML injection) as regression tests for the validators. This is correct security hygiene.

Performance

N/A for unit tests.

Verdict: APPROVE — solid coverage; one advisory gap on dir-blocklist prefix semantics (non-blocking, raised for follow-up).

[core-qa-agent] Five-Axis Review — APPROVED ## Correctness All three validators are well-covered: - **validateWorkspaceID**: valid RFC-4122 UUIDs pass; traversal, SQL injection, short/malformed UUIDs correctly rejected. The all-zeros-no-hyphens case (`"00000000000000000000000000000000"`) correctly fails if the validator requires the hyphenated form. - **validateWorkspaceDir**: relative paths, `~` prefix, traversal sequences (`../`), and all expected system path roots (`/etc`, `/var`, `/proc`, `/sys`, `/dev`, `/boot`, `/sbin`, `/bin`, `/lib`, `/usr`) are tested. Prefix-match tests verify that subdirectories of blocked roots are also rejected. - **validateWorkspaceFields**: length boundaries tested with off-by-one precision (256 fails, 255 passes). Newline/CRLF injection in all four fields tested. YAML special chars blocked in `name`/`role` but permitted in `model`/`runtime` — both sides verified. **Advisory (non-blocking):** `TestValidateWorkspaceDir_PrefixMatchesBlocked` verifies `/etc/molecule-config` is rejected, but does not probe whether a path that *shares characters* with a blocklist entry but is not a subdirectory (e.g. `/variable/workspaces` sharing `/var`) would be a false positive. Depends on whether the implementation uses naive `HasPrefix` or component-level matching. Recommend adding a case like `/variable/workspaces` → should be valid, to pin the correct behavior. **Minor nit:** `TestValidateWorkspaceFields_YAMLCharsAllowedInEmptyName` tests an empty name with a valid role — the name is accurate but slightly misleading. Consider `TestValidateWorkspaceFields_EmptyNameWithValidRole`. ## Readability Clear structure, ASCII dividers, descriptive test names. The `_AllEmpty → valid` comment explaining the design decision (creation uses defaults) is good. ## Architecture In-package, zero external dependencies. Same correct pattern as #744. ## Security Positive: the test file exercises security-relevant inputs (path traversal, SQL injection, YAML injection) as regression tests for the validators. This is correct security hygiene. ## Performance N/A for unit tests. --- **Verdict: APPROVE** — solid coverage; one advisory gap on dir-blocklist prefix semantics (non-blocking, raised for follow-up).

hongming added 1 commit 2026-05-12 20:51:32 +00:00

ci: rerun after mc#724 all-required fix lands

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 15s

Details

sop-tier-check / tier-check (pull_request) Successful in 13s

Details

audit-force-merge / audit (pull_request) Successful in 14s

Details

5e5fb503ec

hongming merged commit 9c37138ac6 into staging

2026-05-12 21:10:29 +00:00

hongming referenced this issue from a commit