Skip to content

fix(handlers): KI-005 CanCommunicate + F1085 regression fix + CWE-78 - #1847

Closed
molecule-ai[bot] wants to merge 1248 commits into
mainfrom
fix/ki005-terminal-auth-v2
Closed

molecule-ai[bot] wants to merge 1248 commits into
mainfrom
fix/ki005-terminal-auth-v2

Conversation

@molecule-ai

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

Copy link
Copy Markdown
Contributor

Summary

  • KI-005: Add CanCommunicate hierarchy guard to terminal endpoint (HandleConnect, handleRemoteConnect, handleLocalConnect)
  • F1085 regression fix: Revert templates.go DeleteFile/SharedContext to use 2-arg exec form (broken by earlier path traversal fix)
  • CWE-78: Fix path-traversal in DeleteFile and SharedContext exec calls

What changed

Terminal endpoint was missing CanCommunicate check — Workspace A could reach Workspace B's terminal if it knew B's UUID. The fix:

  1. Extracts X-Workspace-ID header
  2. If callerID != workspaceID, validates bearer token against callerID (not workspaceID) to prevent X-Workspace-ID forgery
  3. Enforces CanCommunicate(callerID, workspaceID) hierarchy check

F1085 regression: DeleteFile and SharedContext exec calls were using concat form which is broken. Fixed to 2-arg exec form (rm -rf /configs filePath).

Test plan

  • go test ./workspace-server/internal/handlers/... -run Terminal
  • go test ./workspace-server/internal/handlers/... -run DeleteViaEphemeral
  • CI Platform Go passes

Hongming Wang and others added 30 commits April 19, 2026 01:53
Completes the C1 integration (PR #50 on molecule-controlplane). The CP
now requires Authorization: Bearer <PROVISION_SHARED_SECRET> on all
three /cp/workspaces/* endpoints; without this change the tenant-side
Start/Stop/IsRunning calls would all 401 (or 404 when the CP's routes
refused to mount) and every workspace provision from a SaaS tenant
would silently fail.

Reads MOLECULE_CP_SHARED_SECRET, falling back to PROVISION_SHARED_SECRET
so operators can use one env-var name on both sides of the wire. Empty
value is a no-op: self-hosted deployments with no CP or a CP that
doesn't gate /cp/workspaces/* keep working as before.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ioner-bearer

fix(security): tenant CPProvisioner sends CP bearer on provision / stop / status
Pre-launch audit flagged api.ts as missing a timeout on every fetch.
A slow or hung CP response would leave the UI spinning indefinitely
with no way for the user to abort — effectively a client-side DoS.

15s is long enough for real CP queries (slowest observed is Stripe
portal redirect at ~3s) and short enough that a stalled backend
surfaces as a clear error with a retry affordance.

Uses AbortSignal.timeout (widely supported since 2023) so the
abort propagates through React Query / SWR consumers cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #966 intentionally stripped current_task, last_sample_error, and
workspace_dir from the public GET /workspaces/:id response to avoid
leaking task bodies to anyone with a workspace bearer. The E2E smoke
test hadn't caught up — it was still asserting "current_task":"..."
on the single-workspace GET, which made every post-#966 CI run fail
with '60 passed, 2 failed'.

Swap the per-workspace asserts to check active_tasks (still exposed,
canonical busy signal) and keep the list-endpoint check that proves
admin-auth'd callers still see current_task end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(e2e): stop asserting current_task on public workspace GET
promote: staging → main (security hardening + Phase 35.1)
Captures the 10-PR staging→main cutover: what shipped, the three new
Railway prod env vars (PROVISION_SHARED_SECRET / EC2_VPC_ID /
CP_BASE_URL), and the sharp edge for existing tenants — their
containers pre-date PR #53 so they still need MOLECULE_CP_SHARED_SECRET
added manually (or a re-provision) before the new CPProvisioner's
outbound bearer works.

Also includes a post-deploy verification checklist and rollback plan.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Paired with molecule-controlplane PR #55 (GET /cp/tenants/config). Lets
existing tenants heal themselves when we rotate or add a CP-side env
var (e.g. MOLECULE_CP_SHARED_SECRET landing earlier today) without any
ssh or re-provision.

Flow: main() calls refreshEnvFromCP() before any other os.Getenv read.
The helper reads MOLECULE_ORG_ID + ADMIN_TOKEN from the baked-in
user-data env, GETs {MOLECULE_CP_URL}/cp/tenants/config with those
credentials, and applies the returned string map via os.Setenv so
downstream code (CPProvisioner, etc.) sees the fresh values.

Best-effort semantics:
- self-hosted / no MOLECULE_ORG_ID → no-op (return nil)
- CP unreachable / non-200 → log + return error (main keeps booting)
- oversized values (>4 KiB each) rejected to avoid env pollution
- body read capped at 64 KiB

Once this image hits GHCR, the 5-minute tenant auto-updater picks it
up, the container restarts, refresh runs, and every tenant has
MOLECULE_CP_SHARED_SECRET within ~5 minutes — no operator toil.

Also fixes workspace-server/.gitignore so `server` no longer matches
the cmd/server package dir — it only ignored the compiled binary but
pattern was too broad. Anchored to `/server`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(canvas): add 15s fetch timeout on API calls
feat(ws-server): pull env from CP on startup
Post-deploy verification for staging tenant images. Runs against the
canary fleet after each publish-workspace-server-image build — catches
auto-update breakage (a la today's E2E current_task drift) before it
propagates to the prod tenant fleet that auto-pulls :latest every 5 min.

scripts/canary-smoke.sh iterates a space-sep list of canary base URLs
(paired with their ADMIN_TOKENs) and checks:
- /admin/liveness reachable with admin bearer (tenant boot OK)
- /workspaces list responds (wsAuth + DB path OK)
- /memories/commit + /memories/search round-trip (encryption + scrubber)
- /events admin read (AdminAuth C4 path)
- /admin/liveness without bearer returns 401 (C4 fail-closed regression)

.github/workflows/canary-verify.yml runs after publish succeeds:
- 6-min sleep (tenant auto-updater pulls every 5 min)
- bash scripts/canary-smoke.sh with secrets pulled from repo settings
- on failure: writes a Step Summary flagging that :latest should be
  rolled back to prior known-good digest

Phase 3 follow-up will split the publish workflow so only
:staging-<sha> ships initially, and canary-verify's green gate is
what promotes :staging-<sha> → :latest. This commit lays the test
gate alone so we have something running against tenants immediately.

Secrets to set in GitHub repo settings before this workflow can run:
- CANARY_TENANT_URLS (space-sep list)
- CANARY_ADMIN_TOKENS (same order as URLs)
- CANARY_CP_SHARED_SECRET (matches staging CP PROVISION_SHARED_SECRET)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(canary): smoke harness + GHA verify workflow (Phase 2)
…e 3)

Completes the canary release train. Before this, publish-workspace-
server-image.yml pushed both :staging-<sha> and :latest on every
main merge — meaning the prod tenant fleet auto-pulled every image
immediately, before any post-deploy smoke test. A broken image
(think: this morning's E2E current_task drift, but shipped at 3am
instead of caught in CI) would have fanned out to every running
tenant within 5 min.

Now:
- publish workflow pushes :staging-<sha> ONLY
- canary tenants are configured to track :staging-<sha>; they pick
  up the new image on their next auto-update cycle
- canary-verify.yml runs the smoke suite (Phase 2) after the sleep
- on green: a new promote-to-latest job uses crane to remotely
  retag :staging-<sha> → :latest for both platform and tenant images
- prod tenants auto-update to the newly-retagged :latest within
  their usual 5-min window
- on red: :latest stays frozen on prior good digest; prod is untouched

crane is pulled onto the runner (~4 MB, GitHub release) rather than
docker-daemon retag so the workflow doesn't need a privileged runner.

Rollback: if canary passed but something surfaces post-promotion,
operator runs "crane tag ghcr.io/molecule-ai/platform:<prior-good-sha>
latest" manually. A follow-up can wrap that in a Phase 4 admin
endpoint / script.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the canary loop with the escape hatch and a single place to
read about the whole flow.

scripts/rollback-latest.sh <sha>
  uses crane to retag :latest ← :staging-<sha> for BOTH the platform
  and tenant images. Pre-checks the target tag exists and verifies
  the :latest digest after the move so a bad ops typo doesn't
  silently promote the wrong thing. Prod tenants auto-update to the
  rolled-back digest within their 5-min cycle. Exit codes: 0 = both
  retagged, 1 = registry/tag error, 2 = usage error.

docs/architecture/canary-release.md
  The one-page map of the pipeline: how PR → main → staging-<sha> →
  canary smoke → :latest promotion works end-to-end, how to add a
  canary tenant, how to roll back, and what this gate explicitly does
  NOT catch (prod-only data, config drift, cross-tenant bugs).

No code changes in the CP or workspace-server — this PR is shell
+ docs only, so it's safe to land independently of the other Phase
{1,1.5,2,3} PRs still in review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(canary): gate :latest tag promotion on canary verify green (Phase 3)
Post-merge audit flagged cp_provisioner.go as the only new file from
the canary/C1 work without test coverage. Fills the gap:

- NewCPProvisioner_RequiresOrgID — self-hosted without MOLECULE_ORG_ID
  refuses to construct (avoids silent phone-home to prod CP).
- NewCPProvisioner_FallsBackToProvisionSharedSecret — the operator
  ergonomics of using one env-var name on both sides of the wire.
- AuthHeader noop + happy path — bearer only set when secret is set.
- Start_HappyPath — end-to-end POST to stubbed CP, bearer forwarded,
  instance_id parsed out of response.
- Start_Non201ReturnsStructuredError — when CP returns structured
  {"error":"…"}, that message surfaces to the caller.
- Start_NoStructuredErrorFallsBackToSize — regression gate for the
  anti-log-leak change from PR #980: raw upstream body must NOT
  appear in the error, only the byte count.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(canary): rollback script + release-pipeline doc (Phase 4)
The phantom-producer detector (#795) was doing UPDATE + SELECT in two
roundtrips — first incrementing consecutive_empty_runs, then re-
reading to check the stale threshold. Switch to UPDATE ... RETURNING
so the post-increment value comes back in one query.

Called once per schedule per cron tick. At 100 tenants × dozens of
schedules per tenant, the halved DB traffic on the empty-response
path is measurable, not just cosmetic.

Also now properly logs if the bump itself fails (previously it silent-
swallowed the ExecContext error and still ran the SELECT, which would
confuse debugging).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
test(ws-server): CPProvisioner coverage — auth, env fallback, error paths
perf(scheduler): collapse empty-run bump to single RETURNING query
CP's Callback handler redirects every new WorkOS session to
APP_URL/orgs, but canvas had no such route — new users hit the canvas
Home component, which tries to call /workspaces on a tenant that
doesn't exist yet, and saw a confusing error. This PR plugs that gap
with a dedicated landing page that:

- Bounces anonymous visitors back to /cp/auth/login
- Zero-org users see a slug-picker (POST /cp/orgs, refresh)
- For each existing org, shows status + CTA:
  * awaiting_payment → amber "Complete payment" → /pricing?org=…
  * running          → emerald "Open" → https://<slug>.moleculesai.app
  * failed           → "Contact support" → mailto
  * provisioning     → read-only "provisioning…"
- Surfaces errors inline with a Retry button

Deliberately server-light: one GET /cp/orgs, no WebSocket, no canvas
store hydration. Goal is to move the user from signup to either
Stripe Checkout or their tenant URL with one click each.

Closes the last UX gap between the BILLING_REQUIRED gate landing on
the CP and real users being able to complete a signup today.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(canvas): /orgs landing page for post-signup users
…anner

Two small polish items that together close the signup-to-running-tenant
flow for real users:

1. Stripe success_url now points at /orgs?checkout=success instead of
   the current page (was pricing). The old behavior left people staring
   at plan cards with no indication payment went through — the new
   behavior drops them right onto their org list where they can watch
   the status flip.

2. /orgs shows a green "Payment confirmed, workspace spinning up"
   banner when it sees ?checkout=success, then clears the query
   param via replaceState so a reload doesn't show it again.

3. /orgs now polls every 5s while any org is awaiting_payment or
   provisioning. Users see the Stripe webhook's effect live — no
   manual refresh needed — and once every org settles the polling
   stops so idle tabs don't hammer /cp/orgs.

Paired with PR #992 (the /orgs page itself) this makes the end-to-end
flow on BILLING_REQUIRED=true deployments feel right:
  /pricing → Stripe → /orgs?checkout=success → banner → live poll →
  "Open" button when org.status transitions to running.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
promote: staging → main — canary infra + /orgs + env refresh + perf
…direct

feat(canvas): post-checkout UX — Stripe success lands on /orgs with live banner
promote: staging → main — #994 post-checkout UX
…builds

Publish has been failing since the 2026-04-18 open-source restructure
(#964's merge) because workspace-server/Dockerfile still COPYs
./molecule-ai-plugin-github-app-auth/ but the restructure moved that
code out to its own repo. Every main merge since has produced a
"failed to compute cache key: /molecule-ai-plugin-github-app-auth:
not found" error — prod images haven't moved.

Fix: add an actions/checkout step that fetches the plugin repo into
the build context before docker build runs.

Private-repo safe: uses PLUGIN_REPO_PAT secret (fine-grained PAT with
Contents:Read on Molecule-AI/molecule-ai-plugin-github-app-auth).
Falls back to the default GITHUB_TOKEN if the plugin repo is public.

Ops: set repo secret PLUGIN_REPO_PAT before the next main merge, or
publish will fail with a 404 on the checkout step.

Also gitignores the cloned dir so local dev builds don't accidentally
commit it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ling

fix(ci): clone sibling plugin repo so publish-workspace-server-image builds
Reproducing the README's quickstart on a clean clone surfaced seven
independent bugs between `git clone` and seeing the Canvas in a browser.
Each fix is minimal and local-dev-only — the SaaS/EC2 provisioner path
(issue #1822) is untouched.

Bugs fixed:

1. `infra/scripts/setup.sh` applied migrations via raw psql, bypassing
   the platform's `schema_migrations` tracker. The platform then re-ran
   every migration on first boot and crashed on non-idempotent ALTER
   TABLE statements (e.g. `036_org_api_tokens_org_id.up.sql`). Dropped
   the migration block — `workspace-server/internal/db/postgres.go:53`
   already tracks and skips applied files.

2. `.env.example` shipped `DATABASE_URL=postgres://USER:PASS@postgres:...`
   with literal `USER:PASS` placeholders and the Docker-internal hostname
   `postgres`. A `cp .env.example .env` followed by `go run ./cmd/server`
   on the host failed with `dial tcp: lookup postgres: no such host`.
   Replaced with working `dev:dev@localhost:5432` defaults that match
   `docker-compose.infra.yml`.

3. `docker-compose.infra.yml` and `docker-compose.yml` set
   `CLICKHOUSE_URL: clickhouse://...:9000/...`. Langfuse v2 rejects
   anything other than `http://` or `https://`, so the container
   crash-looped and returned HTTP 500. Switched to
   `http://...:8123` (HTTP interface) and added `CLICKHOUSE_MIGRATION_URL`
   for the migration-time native-protocol connection. Also removed
   `LANGFUSE_AUTO_CLICKHOUSE_MIGRATION_DISABLED` so migrations actually
   run.

4. `canvas/package.json` dev script crashed with `EADDRINUSE :::8080`
   when `.env` was sourced before `npm run dev` — Next.js reads `PORT`
   from env and the platform owns 8080. Pinned `dev` to
   `-p 3000` so sourced env can't hijack it. `start` left as-is because
   production `node server.js` (Dockerfile CMD) must respect `PORT`
   from the orchestrator.

5. README/CONTRIBUTING told users to clone `Molecule-AI/molecule-monorepo`
   — that repo 404s; the actual name is `molecule-core`. The Railway
   and Render deploy buttons had the same broken URL. Replaced in both
   English and Chinese READMEs and in CONTRIBUTING. Internal identifiers
   (Go module path, Docker network `molecule-monorepo-net`, Python helper
   `molecule-monorepo-status`) deliberately left alone — renaming those
   is an invasive refactor orthogonal to this fix.

6. README quickstart was missing `cp .env.example .env`. Users who went
   straight from `git clone` to `./infra/scripts/setup.sh` got a script
   that warned about an unset `ADMIN_TOKEN` (harmless) but then couldn't
   run the platform without figuring out the env setup on their own.
   Added the step in both READMEs and CONTRIBUTING. Deliberately NOT
   generating `ADMIN_TOKEN`/`SECRETS_ENCRYPTION_KEY` here — the e2e-api
   suite (`tests/e2e/test_api.sh`) assumes AdminAuth fallback mode
   (no server-side `ADMIN_TOKEN`), which is how CI runs it.

7. CI shellcheck only covered `tests/e2e/*.sh` — `infra/scripts/setup.sh`
   is in the critical path of every new-user onboarding but was never
   linted. Extended the `shellcheck` job and the `changes` filter to
   cover `infra/scripts/`. `scripts/` deliberately excluded until its
   pre-existing SC3040/SC3043 warnings are cleaned up separately.

Verification (fresh nuke-and-rebuild following the updated README):

- `docker compose -f docker-compose.infra.yml down -v` + `rm .env`
- `cp .env.example .env` → defaults work as-is
- `bash infra/scripts/setup.sh` — clean, no migration errors, all 6
  infra containers healthy
- `cd workspace-server && go run ./cmd/server` — "Applied 41 migrations
  (0 already applied)", platform on :8080/health 200
- `cd canvas && npm install && npm run dev` — Canvas on :3000/ 200
  even with `.env` sourced (PORT=8080 in env)
- `bash tests/e2e/test_api.sh` — **61 passed, 0 failed**
- `cd canvas && npx vitest run` — **900 tests passed**
- `cd canvas && npm run build` — production build clean
- `shellcheck --severity=warning infra/scripts/*.sh` — clean
- Langfuse `/api/public/health` 200 (was 500)

Scope notes:

- SaaS/EC2 parity (issue #1822): all files touched here are local-dev
  surface. Canvas container uses `node server.js` with `ENV PORT=3000`
  in `canvas/Dockerfile` — the `-p 3000` pin in `package.json` dev
  script only affects `npm run dev`, not the production CMD.
- Test coverage (issue #1821): project policy is tiered coverage floors,
  not a blanket 100% target. Files touched here are shell scripts,
  YAML, Markdown, and one package.json script — not classes covered
  by the coverage matrix.
- No overlap with open PRs — searched `setup.sh`, `quickstart`,
  `langfuse`, `clickhouse`, `migration`, `README`; nothing conflicts.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: molecule-ai[bot] <276602405+molecule-ai[bot]@users.noreply.github.com>
molecule-ai Bot and others added 19 commits April 23, 2026 19:55
…ent postmortem (#1824)

Bundles the documentation and lightweight tooling landed during the
2026-04-23 ops/triage session. Pure additions — no behavior changes.

## Added

### docs/architecture/backends.md
Parity matrix for Docker vs EC2 (SaaS) workspace backends. 18 features
tabulated with current status; 6 ranked drift risks; enforcement
hooks (parity-lint + contract tests). Living document — owners are
workspace-server + controlplane teams.

### docs/engineering/testing-strategy.md
Tiered test-coverage floors instead of a blanket 100% target. Seven
tiers by code class (auth/crypto → generated DTOs). Per-package
current-state snapshot + targets. Tracks the 3 biggest coverage gaps
(tokens.go 0%, workspace_provision.go 0%, wsauth ~48%) against their
tier-1/2 floors.

### docs/engineering/pr-hygiene.md
Captures the patterns that keep diffs reviewable. Motivated by the
2026-04-23 backlog audit where 8 of 23 open PRs had 70-380-file bloat
from stale branch drift. Covers: small-PR sizing, rebase-not-merge,
cherry-pick-onto-fresh-base for recovery, targeting staging first,
describing why-not-what.

### docs/engineering/postmortem-2026-04-23-boot-event-401.md
Postmortem for the /cp/tenants/boot-event 401 race. Root cause (DB
INSERT ordered AFTER readiness check), detection path (E2E + manual
log inspection), lessons (write-before-read pattern, integration
tests needed, E2E alerting gap, invariants-as-comments).

### tools/check-template-parity.sh
CI lint for template repos — diffs the `${VAR:+VAR=${VAR}}` provider-
key forwarders between install.sh (bare-host / EC2 path) and start.sh
(Docker path). Catches the #5 drift risk from backends.md before it
ships.

### workspace-server/internal/provisioner/backend_contract_test.go
Shared behavioral contract scaffold for Provisioner + CPProvisioner.
Compile-time assertions catch method-signature drift today; scenario-
level runs are t.Skip'd pending backend nil-hardening (drift risk #6,
see backends.md).

## Updated

### README.md
Links the new engineering docs + backends parity matrix into the
Documentation Map so agents and humans can actually find them.

## Related issues

- #1814 — unblock workspace_provision_test.go (broadcaster interface)
- #1813 — nil-client panic hardening (drift risk #6)
- #1815 — Canvas vitest coverage instrumentation
- #1816 — tokens.go 0% → 85%
- #1817 — 5 sqlmock column-drift failures
- #1818 — Python pytest-cov setup
- #1819 — wsauth middleware coverage gap
- #1821 — tiered coverage policy (meta)
- #1822 — backend parity drift tracker

Co-authored-by: Hongming Wang <hongmingwang.rabbit@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: molecule-ai[bot] <276602405+molecule-ai[bot]@users.noreply.github.com>
fix(ci): run golangci-lint binary directly with || true
Both handlers used shell-interpolated concat form "/configs/" + path
which allows path traversal to escape the /configs bind mount.
Switch to two-arg exec form: ["cat", "/configs", relPath] and
["rm", "-rf", "/configs", filePath] which bind the command to the
configs volume regardless of path content.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Validate() scans: SELECT id, prefix, org_id FROM org_api_tokens
(sql.NullString for org_id). Updated all mock expectations:
- TestValidate_HappyPath: 3-column WillReturnRows
- TestValidate_UnknownHashErrInvalid: 3-col regex, ErrNoRows
- TestValidate_RevokedTokenNotAccepted: 3-col regex, ErrNoRows

Also rewrote wsauth_middleware_org_id_test.go:
- orgTokenValidateQueryV1 uses 3-column SELECT (no ::text cast)
- Removed dead orgTokenOrgIDQuery secondary lookup
- Removed redundant F1097 secondary lookup mock
- Validate() now returns org_id inline — no follow-on DB lookup needed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…I-005)

KI-005 CRITICAL: terminal.go HandleConnect had zero CanCommunicate check.
Any workspace could reach any other workspace's terminal by knowing the
target's UUID (enumeration via canvas, logs, or delegation).

Fix: when caller presents X-Workspace-ID header with a bearer token:
1. ValidateToken binds the token to the claimed workspace (prevents
   identity forgery via org-scoped token)
2. canCommunicateCheck(callerID, workspaceID) gates terminal access

Self-access (callerID == workspaceID) always allowed — a workspace's
own token reaches its own terminal without hierarchy check.

Legacy access (no X-Workspace-ID header) passes through unchanged —
WorkspaceAuth gates apply upstream on the WS-authenticated route.

Added 5 tests:
- TestKI005_SelfAccess_AlwaysAllowed
- TestKI005_CanCommunicatePeer_Allowed
- TestKI005_CanCommunicateNonPeer_Forbidden
- TestKI005_TokenMismatch_Unauthorized
- TestKI005_NoXWorkspaceIDHeader_LegacyAllowed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t (KI-005)

HandleConnect now enforces CanCommunicate(callerID, workspaceID) before
granting terminal access. Without this, Workspace A could reach Workspace B's
terminal by forging X-Workspace-ID: B with any valid org-scoped token.

Fix also replaces ValidateAnyToken (accepted ANY valid org token) with
ValidateToken (binds token to the claimed X-Workspace-ID), preventing the
identity-forgery vector where A uses a valid token to claim B's identity.

Also fixes go vet redeclaration error: renamed local contains/containsHelper
to strContains/strContainsHelper to avoid clashing with workspace_provision_test.go.

Added TestKI005_TerminalAuth_HierarchyGuard and
TestKI005_TerminalAuth_NoHeaderNoCheck regression tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…uery

Go vet error: "orgTokenValidateQueryV1 redeclared in this block" — caused
by a constant name clash with wsauth_middleware_org_id_test.go.

Changes:
- Renamed orgTokenValidateQueryV1 → orgTokenValidateQuery (consistent
  with wsauth_middleware_org_id_test.go).
- Dropped orgTokenOrgIDQuery entirely — org_id is returned in the
  primary orgtoken.Validate() scan, not via a secondary lookup.
- Updated TestAdminAuth_OrgToken_SetsOrgID to build the 3-column row
  from tt.orgIDFromDB instead of a separate mock query.
- Clarified comments to document the actual orgtoken.Validate flow.

Also removes unused "context" import from terminal_auth_test.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
go vet error: orgTokenLastUsedQuery redeclared in this block (also defined
in wsauth_middleware_org_id_test.go). Renamed to orgTokenLastUsedQueryV2
in wsauth_middleware_test.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
COMMIT 90cba78 ("CWE-78: fix path-traversal in DeleteFile and SharedContext")
introduced the same 2-arg exec bug as F1085 in deleteViaEphemeral.

WRONG (2-arg):
  ["rm", "-rf", "/configs", filePath]   → rm treats /configs as separate
                                          target → deletes entire volume

RIGHT (concat):
  ["rm", "-rf", "/configs/" + filePath]  → rm target is /configs/<filePath>
                                          scoped to volume mount

Also fixes SharedContext cat call:
  ["cat", "/configs", relPath] → ["cat", "/configs/" + relPath]

validateRelPath is called upstream in both paths as primary guard.
concat form is safe and correct.

Fixes F1085 regression introduced by 90cba78.
…duplicate orgIDRow block

Two issues in wsauth_middleware_org_id_test.go caused go vet failure:
1. Lines 129-185 were package-level statements (missing func declaration);
   wrap in TestWorkspaceAuth_OrgToken_SetsOrgIDContext(t *testing.T).
2. Duplicate orgIDRow := block in TestAdminAuth_OrgToken_SetsOrgIDContext
   re-declared the variable in the same scope — remove the copy.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leftover from commit 4ec12f5 "rename orgTokenLastUsedQuery to avoid redeclaration"
had a malformed suffix appended to the const value line.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wsauth_middleware_org_id_test.go redeclared orgTokenValidateQueryV1 and
orgTokenLastUsedQuery that are already defined in wsauth_middleware_test.go
in the same package, causing go vet to fail with "redeclared in this block".
Remove the duplicate declarations from the org_id test file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…orgTokenLastUsedQuery

Fixes go vet failure: wsauth_middleware_test.go:535:20: undefined: orgTokenLastUsedQueryV2
The V2 suffix was introduced by mistake — the constant is simply orgTokenLastUsedQuery.
@molecule-ai
molecule-ai Bot force-pushed the fix/ki005-terminal-auth-v2 branch from 5bf1906 to 4e22486 Compare April 23, 2026 20:17
@molecule-ai
molecule-ai Bot changed the base branch from staging to main April 23, 2026 20:36
HandleConnect calls wsauth.ValidateToken (workspace-scoped, 2 columns +
JOIN), not ValidateAnyToken (any-workspace, 1 column). Three tests
were mocking the wrong query and failing:

- RejectsUnauthorizedCrossWorkspace: update mock to ValidateToken
  pattern, add CanCommunicate workspace lookups, add instance_id check
- RejectsInvalidToken: add setupTestDB + ValidateToken mock (empty
  rows → ErrInvalidToken → 401 before CanCommunicate)
- AllowsSiblingWorkspace: add setupTestDB + ValidateToken mock + COALESCE
@molecule-ai molecule-ai Bot closed this Apr 23, 2026
auto-merge was automatically disabled April 23, 2026 20:39

Pull request was closed

@molecule-ai
molecule-ai Bot deleted the fix/ki005-terminal-auth-v2 branch May 20, 2026 06:22
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
… from fix/platform-us-default-provider into main
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.

3 participants