From 5037b9619c949341320dd3d3b4c2bbfb677be262 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Mon, 15 Jun 2026 12:24:56 +0000 Subject: [PATCH 1/3] fix(memories): add gho_ GitHub OAuth token prefix and correct ghs_ label (#2921) The GitHub token redaction table was missing the gho_ OAuth user-token prefix and mislabeled ghs_ (GitHub App server-to-server token) as GITHUB_OAUTH. Add gho_ with the correct label and relabel ghs_ to GITHUB_APP_SERVER_TOKEN, plus inline comments documenting the prefix meanings. - Add gho_[A-Za-z0-9]{16,} -> GITHUB_OAUTH. - Change ghs_ label from GITHUB_OAUTH to GITHUB_APP_SERVER_TOKEN. - Add unit tests for both new/changed labels. Fixes #2921 (memory redaction cleanup items). Test plan: - go test ./workspace-server/internal/handlers -run TestRedactSecrets_GitHub - go build ./... --- .../internal/handlers/memories.go | 11 ++++++- .../internal/handlers/memories_test.go | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/workspace-server/internal/handlers/memories.go b/workspace-server/internal/handlers/memories.go index 94f82ba60..679238630 100644 --- a/workspace-server/internal/handlers/memories.go +++ b/workspace-server/internal/handlers/memories.go @@ -68,8 +68,17 @@ var memorySecretPatterns = []secretPatternEntry{ // without an env-var wrapper. Each prefix is provider-specific and // well-known; matching the prefix + 16+ chars of base64-ish body // keeps false-positives low. + // + // GitHub token prefix legend: + // ghp_ → personal access token (classic) + // gho_ → OAuth user token + // ghu_ → user-to-server token + // ghs_ → GitHub App server-to-server token + // ghr_ → refresh token + // github_pat_ → fine-grained PAT {regexp.MustCompile(`\bghp_[A-Za-z0-9]{16,}`), "GITHUB_PAT"}, - {regexp.MustCompile(`\bghs_[A-Za-z0-9]{16,}`), "GITHUB_OAUTH"}, + {regexp.MustCompile(`\bgho_[A-Za-z0-9]{16,}`), "GITHUB_OAUTH"}, + {regexp.MustCompile(`\bghs_[A-Za-z0-9]{16,}`), "GITHUB_APP_SERVER_TOKEN"}, {regexp.MustCompile(`\bghu_[A-Za-z0-9]{16,}`), "GITHUB_USER_TOKEN"}, {regexp.MustCompile(`\bghr_[A-Za-z0-9]{16,}`), "GITHUB_REFRESH_TOKEN"}, {regexp.MustCompile(`\bgithub_pat_[A-Za-z0-9_]{16,}`), "GITHUB_FINEGRAINED_PAT"}, diff --git a/workspace-server/internal/handlers/memories_test.go b/workspace-server/internal/handlers/memories_test.go index f14a44379..9cf7c9bd2 100644 --- a/workspace-server/internal/handlers/memories_test.go +++ b/workspace-server/internal/handlers/memories_test.go @@ -706,6 +706,37 @@ func TestRedactSecrets_GitHubPAT_Raw_IsRedacted(t *testing.T) { } } +func TestRedactSecrets_GitHubOAuth_Raw_IsRedacted(t *testing.T) { + // gho_ is the GitHub OAuth user-token prefix. It was missing from the + // redaction table and was incorrectly lumped under the ghs_ label. + input := "my github oauth: gho_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + out, changed := redactSecrets("ws-1", input) + if !changed { + t.Errorf("raw GitHub OAuth token was not redacted in %q", input) + } + if !strings.Contains(out, "[REDACTED:GITHUB_OAUTH]") { + t.Errorf("expected [REDACTED:GITHUB_OAUTH] in output, got: %q", out) + } + if strings.Contains(out, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") { + t.Errorf("plaintext token leaked through: %q", out) + } +} + +func TestRedactSecrets_GitHubAppServerToken_Raw_IsRedacted(t *testing.T) { + // ghs_ is a GitHub App server-to-server token, not an OAuth token. + input := "github app token: ghs_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + out, changed := redactSecrets("ws-1", input) + if !changed { + t.Errorf("raw GitHub App server-to-server token was not redacted in %q", input) + } + if !strings.Contains(out, "[REDACTED:GITHUB_APP_SERVER_TOKEN]") { + t.Errorf("expected [REDACTED:GITHUB_APP_SERVER_TOKEN] in output, got: %q", out) + } + if strings.Contains(out, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") { + t.Errorf("plaintext token leaked through: %q", out) + } +} + func TestRedactSecrets_GitHubFineGrainedPAT_Raw_IsRedacted(t *testing.T) { // Fine-grained PAT format: github_pat_ + 82+ chars. Test fixture // uses a clearly-fake body (80 chars) that satisfies the -- 2.52.0 From da887ddb633450d12442e13981cea73b08e5cba6 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Mon, 15 Jun 2026 12:35:43 +0000 Subject: [PATCH 2/3] fix(canvas): clear hydrationError on successful hydrate and log failure (#2921) The canvas store set hydrationError on the failure branch but never unset it on the success branch, so a prior failed hydrate left a stale error banner after a subsequent successful rehydrate. Also log the underlying error in the catch block for supportability. - Set hydrationError: null in the hydrate success path. - console.error the caught error before surfacing the user-facing message. - Add a regression test that a successful hydrate clears a previous error. Relates #2921. Test plan: - npm test -- --run -t hydrationError - npm run lint --- canvas/src/store/__tests__/canvas.test.ts | 8 ++++++++ canvas/src/store/canvas.ts | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/canvas/src/store/__tests__/canvas.test.ts b/canvas/src/store/__tests__/canvas.test.ts index 941d979e3..0a5f404a5 100644 --- a/canvas/src/store/__tests__/canvas.test.ts +++ b/canvas/src/store/__tests__/canvas.test.ts @@ -899,6 +899,14 @@ describe("hydrationError", () => { expect(useCanvasStore.getState().nodes).toHaveLength(1); expect(useCanvasStore.getState().nodes[0].id).toBe("ws-x"); }); + + it("successful hydrate clears a previous hydrationError", () => { + useCanvasStore.getState().setHydrationError("previous failure"); + useCanvasStore.getState().hydrate([makeWS({ id: "ws-y", name: "Y" })]); + expect(useCanvasStore.getState().hydrationError).toBeNull(); + expect(useCanvasStore.getState().nodes).toHaveLength(1); + expect(useCanvasStore.getState().nodes[0].id).toBe("ws-y"); + }); }); // ---------- ACTIVITY_LOGGED event ---------- diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index f3c5ee6d1..ab2ecfb57 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -970,11 +970,15 @@ export const useCanvasStore = create((set, get) => ({ layoutOverrides, currentParentSizes, ); - set({ nodes, edges }); + // Clear any stale hydration error from a previous failed load so a + // successful rehydrate does not leave the user staring at an old + // error banner (core#2921). + set({ nodes, edges, hydrationError: null }); } catch (err) { // Fail closed: cyclic/corrupt topology must not hang or blank the app. // Surface a retryable error state and keep the previous nodes so the // user isn't left with an empty canvas. + console.error("Canvas hydration failed:", err); const message = err instanceof TopologyCycleError ? `Workspace map has a cyclic parent chain: ${err.message}. Please reload or contact support.` -- 2.52.0 From 16a8e5d740ac5c46977ea2d2907f471dd58a01be Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Mon, 15 Jun 2026 12:41:21 +0000 Subject: [PATCH 3/3] test(memories): add dynamic AWS access-key ID redaction tests (#2921) Adds runtime-built test fixtures for the AKIA[A-Z0-9]{16} redaction pattern so the pre-commit secret scanner does not flag the test source. - Verifies a 20-char AKIA-prefixed key is redacted as AWS_ACCESS_KEY_ID. - Verifies an AKID-prefixed string is not false-positived. Relates #2921. --- .../internal/handlers/memories_test.go | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/workspace-server/internal/handlers/memories_test.go b/workspace-server/internal/handlers/memories_test.go index 9cf7c9bd2..b92775012 100644 --- a/workspace-server/internal/handlers/memories_test.go +++ b/workspace-server/internal/handlers/memories_test.go @@ -752,17 +752,32 @@ func TestRedactSecrets_GitHubFineGrainedPAT_Raw_IsRedacted(t *testing.T) { } } -// TestRedactSecrets_AWSAccessKeyID_Raw_IsRedacted is intentionally NOT -// added as a string-literal test — the redaction pattern is -// `AKIA[A-Z0-9]{16}` (exactly 16 uppercase/digit chars, matching AWS -// access key IDs which are always 20 chars total). Any test fixture -// that satisfies the redaction pattern ALSO matches the pre-commit -// secret scanner's `AKIA[0-9A-Z]{16}` rule (same 16-char body), so a -// test literal cannot demonstrate redaction without triggering a -// false-positive on the local secret scanner. The pattern is -// self-evidently correct from inspection; the redaction label -// `AWS_ACCESS_KEY_ID` is verified by manual verification of the -// pattern list in memories.go. +// TestRedactSecrets_AWSAccessKeyID exercises the AKIA[A-Z0-9]{16} pattern +// without using a single string literal that would trip the pre-commit secret +// scanner. The test fixture builds the candidate dynamically. +func TestRedactSecrets_AWSAccessKeyID_Raw_IsRedacted(t *testing.T) { + input := "aws access key: " + "AKIA" + strings.Repeat("A", 16) + out, changed := redactSecrets("ws-1", input) + if !changed { + t.Errorf("AWS access key ID was not redacted in %q", input) + } + if !strings.Contains(out, "[REDACTED:AWS_ACCESS_KEY_ID]") { + t.Errorf("expected [REDACTED:AWS_ACCESS_KEY_ID] in output, got: %q", out) + } + if strings.Contains(out, "AKIA") { + t.Errorf("plaintext AWS access key ID leaked through: %q", out) + } +} + +func TestRedactSecrets_AWSAccessKeyID_AKIDPrefix_PassesThrough(t *testing.T) { + // AKID is a documented AWS access-key prefix in some docs but is NOT the + // IAM access-key prefix matched by the scanner. Ensure we don't false-positive. + input := "not-an-akid: " + "AKID" + strings.Repeat("A", 16) + out, changed := redactSecrets("ws-1", input) + if changed && strings.Contains(out, "[REDACTED:AWS_ACCESS_KEY_ID]") { + t.Errorf("AKID-prefixed string was incorrectly redacted as AWS_ACCESS_KEY_ID: %q", out) + } +} func TestRedactSecrets_VercelToken_Raw_IsRedacted(t *testing.T) { input := "vercel token: vc_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -- 2.52.0