Skip to content

fix(security): #226 — gate POST /workspaces template against traversal - #233

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/issue-226-create-template-traversal
Apr 15, 2026
Merged

fix(security): #226 — gate POST /workspaces template against traversal#233
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/issue-226-create-template-traversal

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Closes #226 MEDIUM. Same pattern as #103 (resolveInsideRoot). Two call sites in workspace.go, both guarded.

…traversal

Closes #226 MEDIUM. WorkspaceHandler.Create joined payload.Template
directly into filepath.Join(configsDir, template) without validating
it stayed inside configsDir. An attacker posting Template="../../etc"
would have the provisioner walk and mount arbitrary host directories
into the workspace container.

Same fix as #103 (POST /org/import): use the existing resolveInsideRoot
helper to reject absolute paths and any ".." that escapes the root.
Applied at both call sites in workspace.go:
  1. Synchronous runtime detection before DB insert — 400 on bad input
  2. Async provisioning goroutine — early return, logs the rejection
     (belt-and-suspenders; the create path already blocks)

No test added inline because the existing resolveInsideRoot suite
(org_path_test.go) already covers absolute / traversal / prefix-sibling
/ empty-path / deep-subpath cases. A duplicate test for the workspace
handler wouldn't add signal.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit ded41c2 into main Apr 15, 2026
HongmingWang-Rabbit pushed a commit that referenced this pull request Apr 15, 2026
…ht sweep

Captures ~27 PRs merged across both repos this session: security
hardening cluster (#94/#99/#106/#110/#119/#162/#155/#167/#185/#200/#203/
#209/#233), data-integrity fixes (#212/#224/#236), CI runner migration
(#186), platform/scheduler reliability (#95/#149/#207/#206), workspace
runtime features (#205/#208/#198/#216/#225/#235/#231), code-review
follow-ups (#228/#232).

Updated counts: 816 Go (+70), 1180 Python (+40), 453 vitest (unchanged
— UI/a11y patches), 97 jest (unchanged).

CLAUDE.md additions:
- Idle Loop section (#205) under Architectural Patterns
- Admin auth middleware variants section linking docs/runbooks/admin-auth.md
- Migration runner section explaining the .down.sql filter (#212)
- Per-route auth notes in the API table (PATCH field-whitelist, CanvasOrBearer
  on PUT /canvas/viewport, AdminAuth on bundles/events/templates-import/
  approvals-pending/admin-liveness)
- Database section updated with workspace_auth_tokens auto-revoke (#110),
  scheduler.error_detail surfacing (#206), workspace_schedules.last_status
  'skipped' state (#207)

PLAN.md additions:
- New Recently launched (overnight sweep) section with full PR/issue index
- Phase status updated (B–G now complete, H partial)
- Live infrastructure deltas (migration fix, token rotation, legal pages)
- Outstanding items consolidated

Edit-history file expanded from the tick-9 stub to a full session record
covering malware cleanup, CI runner migration, security cluster, data
integrity, infra/feature/code-review batches, and outstanding user
actions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Apr 15, 2026
)

Closes #248. Three instances of the same YAML-injection bug class
(#221 name/role, #233 template path, #241 runtime/model) shipped in
this repo over the last weeks. The common root cause is the Security
Auditor's system prompt didn't list YAML injection as an explicit
check class, so audits missed the pattern every time.

Adds:
- "YAML injection" to the 'Think like an attacker' list in How You Work
- An explicit entry in What You Check with the three prior instances
  cited so future auditors see the pattern and the fix shape
  (double-quoted scalars or a proper YAML encoder)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/issue-226-create-template-traversal branch April 16, 2026 12:31
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
…e-traversal

fix(security): #226 — gate POST /workspaces template against traversal
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
)

Closes #248. Three instances of the same YAML-injection bug class
(#221 name/role, #233 template path, #241 runtime/model) shipped in
this repo over the last weeks. The common root cause is the Security
Auditor's system prompt didn't list YAML injection as an explicit
check class, so audits missed the pattern every time.

Adds:
- "YAML injection" to the 'Think like an attacker' list in How You Work
- An explicit entry in What You Check with the three prior instances
  cited so future auditors see the pattern and the fix shape
  (double-quoted scalars or a proper YAML encoder)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
…ht sweep

Captures ~27 PRs merged across both repos this session: security
hardening cluster (#94/#99/#106/#110/#119/#162/#155/#167/#185/#200/#203/
#209/#233), data-integrity fixes (#212/#224/#236), CI runner migration
(#186), platform/scheduler reliability (#95/#149/#207/#206), workspace
runtime features (#205/#208/#198/#216/#225/#235/#231), code-review
follow-ups (#228/#232).

Updated counts: 816 Go (+70), 1180 Python (+40), 453 vitest (unchanged
— UI/a11y patches), 97 jest (unchanged).

CLAUDE.md additions:
- Idle Loop section (#205) under Architectural Patterns
- Admin auth middleware variants section linking docs/runbooks/admin-auth.md
- Migration runner section explaining the .down.sql filter (#212)
- Per-route auth notes in the API table (PATCH field-whitelist, CanvasOrBearer
  on PUT /canvas/viewport, AdminAuth on bundles/events/templates-import/
  approvals-pending/admin-liveness)
- Database section updated with workspace_auth_tokens auto-revoke (#110),
  scheduler.error_detail surfacing (#206), workspace_schedules.last_status
  'skipped' state (#207)

PLAN.md additions:
- New Recently launched (overnight sweep) section with full PR/issue index
- Phase status updated (B–G now complete, H partial)
- Live infrastructure deltas (migration fix, token rotation, legal pages)
- Outstanding items consolidated

Edit-history file expanded from the tick-9 stub to a full session record
covering malware cleanup, CI runner migration, security cluster, data
integrity, infra/feature/code-review batches, and outstanding user
actions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…0821)

CR2 RC 10821: lint-required-context-exists-in-bp FAILS because the
new `reserved-path-review` status emission (added by the gate
introduced in CR2 10782 + the base-exec fix in this same PR) lacks
the adjacent bp-required / bp-exempt directive. Default on a new
emitter = FAIL with a 3-option fix-hint.

Also: the arm64-pilot shellcheck job in lint-shellcheck-arm64-pilot.yml
emits `Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot)` and
is missing its directive (the lint flags it for the same reason).

FIXES:

  - reserved-path-review.yml: add `# bp-required: pending #673` adjacent
    to the reserved-path-review job. Rationale: the self-merge guard IS
    intended to be branch-protection-enforced (that's its purpose —
    closes cp#673). But main BP currently has 0 reserved-path-review
    in status_check_contexts (only CI / all-required / E2E API Smoke /
    Handlers PG are pinned in .gitea/required-contexts.txt), so
    `bp-required: yes` would fail the in-BP verification. Therefore
    `bp-required: pending #673`: commit to enforcement, defer the
    actual BP-status-check-add to a separate operator follow-up
    (tracked via cp#673). The operator action is OUT OF SCOPE for
    the lint and this PR.

  - lint-shellcheck-arm64-pilot.yml: add `# bp-exempt: arm64-pilot
    shellcheck lane` adjacent to the shellcheck-arm64 job. The arm64
    pilot is non-gating by design (internal#494, #233) — additive fast
    signal on the Mac mini runner, never blocks a merge. Pilot must
    not make main red (#2146). (Sibling ci-arm64-advisory.yml already
    has its bp-exempt directive; this fills the second arm64-pilot
    lane the lint flags.)

NO production code change. The reserved-path-review gate logic and
the shellcheck arm64-pilot logic are both untouched. The script is
already un-tamperable (base-exec fix in prior commit c07e838c);
this commit just makes the workflow's NEW status emission
lint-compliant so lint-required-context-exists-in-bp goes green.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security(path-traversal): POST /workspaces Template/Runtime fields escape configsDir without validation

1 participant