address greptile review feedback (greploop iteration 2)

- storageUtils: replace deprecated escape()/unescape() with
  TextEncoder/TextDecoder for UTF-8 base64 encoding
- chatHistory: use setObfuscated/getObfuscated consistently across
  ChatUI.tsx, useChatHistory.ts, and tests
- Tests updated and passing (39/39)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
yuneng-jiang 2026-03-23 23:23:43 -07:00
parent d5bad2f53e
commit 7356bf1e48
4 changed files with 35 additions and 14 deletions

View file

@ -347,7 +347,7 @@ const ChatUI: React.FC<ChatUIProps> = ({
useEffect(() => {
if (simplified) return; // Do not persist chat history in simplified (embedded) mode
const handler = setTimeout(() => {
sessionStorage.setItem("chatHistory", JSON.stringify(chatHistory));
setObfuscated("chatHistory", JSON.stringify(chatHistory));
}, 500); // Debounce by 500ms
return () => {

View file

@ -1,6 +1,7 @@
import { renderHook, act } from "@testing-library/react";
import { describe, it, expect, beforeEach, vi } from "vitest";
import { useChatHistory } from "./useChatHistory";
import { setObfuscated, getObfuscated } from "../../../utils/storageUtils";
describe("useChatHistory", () => {
beforeEach(() => {
@ -439,7 +440,7 @@ describe("useChatHistory", () => {
vi.useFakeTimers();
const { result } = renderHook(() => useChatHistory({ simplified: false }));
sessionStorage.setItem("chatHistory", "[]");
setObfuscated("chatHistory", "[]");
sessionStorage.setItem("messageTraceId", "trace-1");
sessionStorage.setItem("responsesSessionId", "resp-1");
@ -452,7 +453,7 @@ describe("useChatHistory", () => {
vi.advanceTimersByTime(600);
});
expect(sessionStorage.getItem("chatHistory")).toBeNull();
expect(getObfuscated("chatHistory")).toBeNull();
expect(sessionStorage.getItem("messageTraceId")).toBeNull();
expect(sessionStorage.getItem("responsesSessionId")).toBeNull();
@ -460,7 +461,7 @@ describe("useChatHistory", () => {
});
it("should NOT clear sessionStorage when simplified", () => {
sessionStorage.setItem("chatHistory", '[{"role":"user","content":"hi"}]');
setObfuscated("chatHistory", '[{"role":"user","content":"hi"}]');
const { result } = renderHook(() => useChatHistory({ simplified: true }));
@ -469,7 +470,7 @@ describe("useChatHistory", () => {
});
// simplified mode should not touch sessionStorage
expect(sessionStorage.getItem("chatHistory")).toBe('[{"role":"user","content":"hi"}]');
expect(getObfuscated("chatHistory")).toBe('[{"role":"user","content":"hi"}]');
});
it("should not re-write chatHistory to sessionStorage after clear via debounce", () => {
@ -485,7 +486,7 @@ describe("useChatHistory", () => {
act(() => {
vi.advanceTimersByTime(600);
});
expect(sessionStorage.getItem("chatHistory")).not.toBeNull();
expect(getObfuscated("chatHistory")).not.toBeNull();
// Now clear
act(() => {
@ -497,7 +498,7 @@ describe("useChatHistory", () => {
vi.advanceTimersByTime(600);
});
expect(sessionStorage.getItem("chatHistory")).toBeNull();
expect(getObfuscated("chatHistory")).toBeNull();
vi.useRealTimers();
});

View file

@ -2,6 +2,7 @@ import React, { useState, useEffect } from "react";
import { MessageType, A2ATaskMetadata } from "./types";
import { TokenUsage } from "./ResponseMetrics";
import { MCPEvent } from "../../mcp_tools/types";
import { getObfuscated, setObfuscated } from "../../../utils/storageUtils";
import { truncateString } from "../../../utils/textUtils";
export interface UseChatHistoryReturn {
@ -40,7 +41,7 @@ export function useChatHistory({ simplified }: { simplified: boolean }): UseChat
const [chatHistory, setChatHistory] = useState<MessageType[]>(() => {
if (simplified) return [];
try {
const saved = sessionStorage.getItem("chatHistory");
const saved = getObfuscated("chatHistory");
return saved ? JSON.parse(saved) : [];
} catch (error) {
console.error("Error parsing chatHistory from sessionStorage", error);
@ -71,7 +72,7 @@ export function useChatHistory({ simplified }: { simplified: boolean }): UseChat
// don't re-write an empty array back into sessionStorage.
if (chatHistory.length === 0) return;
const handler = setTimeout(() => {
sessionStorage.setItem("chatHistory", JSON.stringify(chatHistory));
setObfuscated("chatHistory", JSON.stringify(chatHistory));
}, 500); // Debounce by 500ms
return () => {

View file

@ -8,12 +8,31 @@
* storage of sensitive data (CodeQL js/clear-text-storage-of-sensitive-data).
*/
/**
* Encode a UTF-8 string to base64 without using the deprecated
* `escape()` / `unescape()` helpers. Works for any Unicode input.
*/
function utf8ToBase64(str: string): string {
const bytes = new TextEncoder().encode(str);
let binary = "";
for (const b of bytes) {
binary += String.fromCharCode(b);
}
return btoa(binary);
}
/**
* Decode a base64 string back to the original UTF-8 string.
*/
function base64ToUtf8(b64: string): string {
const binary = atob(b64);
const bytes = Uint8Array.from(binary, (ch) => ch.charCodeAt(0));
return new TextDecoder().decode(bytes);
}
export function setObfuscated(key: string, value: string): void {
try {
// Encode via encodeURIComponent first so non-Latin1 characters
// (e.g. Unicode MCP server aliases in OAuth flow-state JSON)
// are converted to percent-encoded ASCII before btoa.
sessionStorage.setItem(key, btoa(unescape(encodeURIComponent(value))));
sessionStorage.setItem(key, utf8ToBase64(value));
} catch {
// quota exceeded or SSR — silently drop
}
@ -23,7 +42,7 @@ export function getObfuscated(key: string): string | null {
try {
const raw = sessionStorage.getItem(key);
if (raw === null) return null;
return decodeURIComponent(escape(atob(raw)));
return base64ToUtf8(raw);
} catch {
// invalid base64 or SSR — treat as missing
return null;