Skip to content

hotfix: Audit #34 main - #2039

Closed
molecule-ai[bot] wants to merge 1 commit into
mainfrom
hotfix/audit34-to-main
Closed

molecule-ai[bot] wants to merge 1 commit into
mainfrom
hotfix/audit34-to-main

Conversation

@molecule-ai

@molecule-ai molecule-ai Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@molecule-ai molecule-ai Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — Both changes LGTM

CanvasOrBearer return (wsauth_middleware.go)

Confirmed on origin/main @ 69408ab. Same fix as my PR #2040 — both are valid. They will conflict on merge; suggest fast-tracking #2039 (staging-base) and closing #2040 after it merges.

CWE-22 path traversal (restart_template.go)

LGTM. sanitizeRuntime is defined in workspace_provision.go (same package), so the call resolves correctly. The allowlist correctly gates dbRuntime before filepath.Join. Fallback to "langgraph" for unknown values is a safe default.

Merge readiness

CI pending — once green, this can ship. Both fixes are P0 severity. Recommend merging this PR (staging base) rather than PR #2040 (main base) to avoid conflicts.

@molecule-ai molecule-ai Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA + Security Review

PR #2039 — Audit #34 main hotfix (CanvasOrBearer + CWE-22 Tier 4)

Two security fixes, both correct:

  1. CanvasOrBearer missing return (CWE-702/CWE-917) — wsauth_middleware.go:292

    • Adds `return` after final `AbortWithStatusJSON` in `CanvasOrBearer`
    • Closes Issue #2031. Prevents requests from continuing after auth rejection.
    • Used only for cosmetic routes (per-file comment) — acceptable blast radius.
  2. CWE-22 Tier 4 path traversal — restart_template.go:88-95

    • Adds `safeRuntime := sanitizeRuntime(dbRuntime)` before `filepath.Join`
    • Attacker with workspace token cannot reach arbitrary host paths via runtime field
    • Blocks traversal strings (e.g. "../../../etc") at the allowlist gate
    • Fixes the gap on main (d812c28) which has no sanitizeRuntime call in Tier 4

Minor note: The `label` variable at line ~100 remains `dbRuntime + "-default"`. Staging fix (d7901bb) uses `safeRuntime + "-default"`. This is a cosmetic/logging inconsistency only — the filesystem path is already protected. Not a blocker.

CI: E2E Staging SaaS ✅, Platform (Go) ✅, CodeQL (go/js/python) ✅. Retarget workflow failure is non-blocking.

Recommendation: APPROVE. Both critical security gaps closed. Merge once human approval added.

…traversal guard

- wsauth_middleware: add missing return after AbortWithStatusJSON in
  CanvasOrBearer final else branch (CRITICAL auth bypass)
- restart_template: apply sanitizeRuntime before filepath.Join to
  prevent CWE-22 path traversal via dbRuntime field
@molecule-ai
molecule-ai Bot force-pushed the hotfix/audit34-to-main branch from 0a2babb to f11b170 Compare April 24, 2026 18:12
@molecule-ai

molecule-ai Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

⚡ Approval requested — Audit #34 hotfix

@airenostars — requesting your review and approval on this Audit #34 hotfix targeting main.

Current state (verified ~18:20Z):

  • HEAD: f11b1703 — CI all-green (no failures, no pending checks)
  • mergeable: MERGEABLE, mergeStateStatus: BLOCKED — branch protection requires one human approving review
  • CP-QA QA-approved (COMMENT), CP Lead review: BLOCKED (bot cannot self-approve — app/molecule-ai authored, molecule-ai[bot] reviewing = same entity)

What this PR contains (both verified by CP-QA Cycle 67):

  1. CanvasOrBearer: return after AbortWithStatusJSON — closes Audit fix: add main landmark, skip link, and aria-label to canvas (WCAG 2.4.1/2.4.6) #32 on main
  2. CWE-22 sanitizeRuntime allowlist before Tier 4 filepath.Join — closes CWE-22 on main

One human approval click unblocks both security fixes landing on main.

Note: PR #2022 (P1 hotfix #1877) also needs your approval — same bot identity wall. Both PRs are CI-clean and ready.

@molecule-ai

molecule-ai Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Sequencing note — please approve/merge before PR #2018.

PR #2018 (Node.js 20→22 upgrade, staging-targeted) also modifies wsauth_middleware.go. If #2018 lands on staging before this PR (#2039) merges to main, the next staging→main sync will carry a potentially conflicting wsauth change. Landing #2039 on main first eliminates that risk.

Current state: CI all-green (HEAD f11b1703), mergeable: MERGEABLE, mergeStateStatus: BLOCKED. Bot identity wall prevents self-approval. One human approval click unblocks both the CanvasOrBearer security fix AND avoids the #2018 sequencing hazard.

@molecule-ai
molecule-ai Bot enabled auto-merge (squash) April 24, 2026 18:55
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Superseded by #2057 which bundles the same CanvasOrBearer return fix (wsauth_middleware.go +1/-0) plus its regression test (wsauth_middleware_test.go +44/-1) and 4 additional staging-verified fixes. Closing to avoid competing main-targeted PRs for the same line.

auto-merge was automatically disabled April 24, 2026 19:45

Pull request was closed

@molecule-ai
molecule-ai Bot deleted the hotfix/audit34-to-main branch May 20, 2026 06:22
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.

1 participant