diff --git a/canvas/src/app/orgs/page.tsx b/canvas/src/app/orgs/page.tsx index 5f1787d6..29a32632 100644 --- a/canvas/src/app/orgs/page.tsx +++ b/canvas/src/app/orgs/page.tsx @@ -352,7 +352,8 @@ function CreateOrgForm({ onCreated }: { onCreated: (slug: string) => void }) { }); if (!res.ok) { const body = await res.text(); - throw new Error(`${res.status}: ${body}`); + console.error(`[orgs] create ${res.status}: ${body}`); + throw new Error(`Failed to create organization (${res.status})`); } onCreated(slug); } catch (e) { diff --git a/canvas/src/lib/__tests__/billing.test.ts b/canvas/src/lib/__tests__/billing.test.ts index 9572296c..69c0e1bc 100644 --- a/canvas/src/lib/__tests__/billing.test.ts +++ b/canvas/src/lib/__tests__/billing.test.ts @@ -70,7 +70,7 @@ describe("startCheckout", () => { expect(body.cancel_url).toContain("checkout=cancel"); }); - it("throws with the body text on non-2xx so the UI can surface it", async () => { + it("throws with status code on non-2xx; body is logged not surfaced", async () => { (global.fetch as ReturnType).mockResolvedValue({ ok: false, status: 402, @@ -78,8 +78,11 @@ describe("startCheckout", () => { json: async () => ({}), }); + // Status code must appear so callers know what happened. await expect(startCheckout("starter", "acme")).rejects.toThrow(/402/); - await expect(startCheckout("starter", "acme")).rejects.toThrow(/payment required/); + // Body text must NOT appear — it may contain Stripe API detail. + await expect(startCheckout("starter", "acme")).rejects.toThrow(/checkout failed/); + await expect(startCheckout("starter", "acme")).rejects.not.toThrow(/payment required/); }); it("sends users to /orgs on success, back to current page on cancel", async () => { diff --git a/canvas/src/lib/billing.ts b/canvas/src/lib/billing.ts index 35f9833d..c9260e61 100644 --- a/canvas/src/lib/billing.ts +++ b/canvas/src/lib/billing.ts @@ -120,8 +120,12 @@ export async function startCheckout( }), }); if (!res.ok) { - const text = await res.text(); - throw new Error(`checkout: ${res.status} ${text}`); + // Never embed res.text() in the thrown error — the response body + // may contain Stripe API error detail (e.g. invalid key, card decline + // message, raw Stripe envelope) that should not reach the client. + const detail = await res.text(); + console.error(`[billing] checkout ${res.status}: ${detail}`); + throw new Error(`checkout failed (${res.status})`); } return res.json(); } @@ -141,8 +145,12 @@ export async function openBillingPortal(orgSlug: string): Promise { body: JSON.stringify({ org_slug: orgSlug, return_url: returnUrl }), }); if (!res.ok) { - const text = await res.text(); - throw new Error(`portal: ${res.status} ${text}`); + // Never embed res.text() in the thrown error — the response body + // may contain Stripe API error detail (e.g. invalid key, card decline + // message, raw Stripe envelope) that should not reach the client. + const detail = await res.text(); + console.error(`[billing] portal ${res.status}: ${detail}`); + throw new Error(`portal failed (${res.status})`); } const data = (await res.json()) as { url: string }; return data.url; diff --git a/workspace-server/internal/handlers/admin_test_token.go b/workspace-server/internal/handlers/admin_test_token.go index 6a2bb9c6..ea005688 100644 --- a/workspace-server/internal/handlers/admin_test_token.go +++ b/workspace-server/internal/handlers/admin_test_token.go @@ -18,10 +18,12 @@ package handlers import ( + "crypto/subtle" "database/sql" "log" "net/http" "os" + "strings" "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" "github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth" @@ -55,6 +57,20 @@ func (h *AdminTestTokenHandler) GetTestToken(c *gin.Context) { return } + // IDOR fix (#112, CRITICAL): when ADMIN_TOKEN is set, require it + // explicitly. Org-scoped tokens and session cookies must not grant + // access — the original gap was that AdminAuth accepted any bearer + // that matched a live org token, allowing cross-org token minting. + adminSecret := os.Getenv("ADMIN_TOKEN") + if adminSecret != "" { + tok := c.GetHeader("Authorization") + tok = strings.TrimPrefix(tok, "Bearer ") + if tok == "" || subtle.ConstantTimeCompare([]byte(tok), []byte(adminSecret)) != 1 { + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "admin auth required"}) + return + } + } + workspaceID := c.Param("id") if workspaceID == "" { c.JSON(http.StatusNotFound, gin.H{"error": "not found"}) diff --git a/workspace-server/internal/handlers/org_plugin_allowlist.go b/workspace-server/internal/handlers/org_plugin_allowlist.go index 99672b03..ff58d6ac 100644 --- a/workspace-server/internal/handlers/org_plugin_allowlist.go +++ b/workspace-server/internal/handlers/org_plugin_allowlist.go @@ -105,6 +105,61 @@ type putAllowlistRequest struct { EnabledBy string `json:"enabled_by"` // workspace ID of the admin performing the change } +// requireCallerOwnsOrg returns the caller's org workspace ID from the +// request context, or "" if the caller is not an org-token holder. +// Used to enforce org isolation on org-scoped routes — the org_token_id +// is the org-scoped token's ID (not the org workspace ID), so we look +// it up via the created_by workspace or a direct relationship. +// +// Returns ("", nil) when the caller is a session/ADMIN_TOKEN user (they +// bypass via the session cookie path or ADMIN_TOKEN, not org tokens). +func requireCallerOwnsOrg(c *gin.Context) (string, error) { + tokenID, ok := c.Get("org_token_id") + if !ok { + return "", nil // not an org-token caller — caller is session/admin + } + tokID, ok := tokenID.(string) + if !ok || tokID == "" { + return "", nil + } + // Look up the org workspace that owns this token. + // org_api_tokens has no org_id column, but we can look up the token's + // created_by workspace and treat that as the caller's org anchor. + var createdBy string + err := db.DB.QueryRowContext(c.Request.Context(), + `SELECT created_by FROM org_api_tokens WHERE id = $1`, tokID, + ).Scan(&createdBy) + if err != nil || createdBy == "" { + // Token has no created_by (CLI bootstrap path) — treat as unowned; + // deny by default to prevent cross-org access. + return "", fmt.Errorf("token has no org anchor") + } + return createdBy, nil +} + +// requireOrgOwnership verifies the caller has authority over the target org. +// Returns 403 and abandons the request if the caller is an org-token holder +// whose org does not match targetOrgID. +func requireOrgOwnership(c *gin.Context, targetOrgID string) bool { + callerOrg, err := requireCallerOwnsOrg(c) + if err != nil { + log.Printf("allowlist: requireOrgOwnership: %v", err) + c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": "org access denied"}) + return false + } + // callerOrg "" means session/admin user — they have full access (no + // org token → full platform admin via session/ADMIN_TOKEN path). + if callerOrg == "" { + return true + } + if callerOrg != targetOrgID { + log.Printf("allowlist: org-token org %s tried to access org %s (denied)", callerOrg, targetOrgID) + c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": "org access denied"}) + return false + } + return true +} + // GetAllowlist handles GET /orgs/:id/plugins/allowlist. // // Returns the current allowlist for the org workspace identified by :id. @@ -128,6 +183,13 @@ func (h *OrgPluginAllowlistHandler) GetAllowlist(c *gin.Context) { return } + // IDOR fix (#112, HIGH): org-token holders must only access their own org. + // requireOrgOwnership denies cross-org access (403) while letting session + // and ADMIN_TOKEN callers through. + if !requireOrgOwnership(c, orgID) { + return + } + rows, err := db.DB.QueryContext(ctx, ` SELECT plugin_name, enabled_by, enabled_at FROM org_plugin_allowlist @@ -210,6 +272,11 @@ func (h *OrgPluginAllowlistHandler) PutAllowlist(c *gin.Context) { return } + // IDOR fix (#112, HIGH): same as GetAllowlist — require org ownership. + if !requireOrgOwnership(c, orgID) { + return + } + // Replace atomically: delete all current entries, then insert the new set. tx, err := db.DB.BeginTx(ctx, nil) if err != nil {