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.` 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..b92775012 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 @@ -721,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"