fix: harden token handling and key rotation logs (#1999)

This commit is contained in:
Jarl
2026-05-17 15:20:10 -07:00
committed by GitHub
parent 4738ef1c8c
commit 3aaba5e30c
6 changed files with 166 additions and 26 deletions
+1 -13
View File
@@ -458,25 +458,13 @@ export default function CoreStateProvider({ children }: { children: ReactNode })
const onSessionTokenUpdated = (event: Event) => {
const customEvent = event as CustomEvent<{ sessionToken?: string | null }>;
const token = customEvent.detail?.sessionToken;
if (!token) {
if (typeof token !== 'string' || token.length === 0) {
return;
}
snapshotRequestIdRef.current += 1;
logoutGuardUntilRef.current = 0;
memoryTokenRef.current = token;
commitState(previous => ({
...previous,
isBootstrapping: false,
isReady: true,
snapshot: {
...previous.snapshot,
auth: { ...previous.snapshot.auth, isAuthenticated: true },
sessionToken: token,
},
}));
void refresh().catch(err => {
log('refresh failed after deep-link session update: %O', sanitizeError(err));
});
@@ -371,6 +371,34 @@ describe('CoreStateProvider — identity-change cache clearing', () => {
expect(vi.mocked(tauriCommands.logout)).toHaveBeenCalledTimes(1);
});
it('ignores forged session-token-updated events that do not match the core snapshot (#1937)', async () => {
fetchSnapshot.mockResolvedValue(makeSnapshot({ userId: 'u1', sessionToken: 'tok1' }));
listTeams.mockResolvedValue([]);
render(
<CoreStateProvider>
<Consumer />
</CoreStateProvider>
);
await waitFor(() => expect(screen.getByTestId('token').textContent).toBe('tok1'));
// Keep the follow-up refresh pending so this assertion observes the
// event handler itself. A forged event must not be able to replace the
// in-memory auth token before refreshCore re-pulls authoritative state.
fetchSnapshot.mockImplementation(() => new Promise(() => {}) as never);
await act(async () => {
window.dispatchEvent(
new CustomEvent('core-state:session-token-updated', {
detail: { sessionToken: 'attacker-controlled-token' },
})
);
});
expect(screen.getByTestId('token').textContent).toBe('tok1');
});
it('setMeetAutoOrchestratorHandoff swallows refresh errors after the RPC succeeds (#1299)', async () => {
fetchSnapshot.mockResolvedValueOnce(makeSnapshot({ userId: 'u1', sessionToken: 'tok1' }));
listTeams.mockResolvedValue([]);
@@ -90,4 +90,38 @@ describe('userScopedStorage', () => {
setItemSpy.mockRestore();
}
});
test('userScopedStorage redacts auth/session tokens before persisting redux blobs (#1938)', async () => {
const mod = await importModule();
mod.primeActiveUserId('user-123');
const persistedReduxBlob = JSON.stringify({
sessionToken: 'top-level-token',
auth: JSON.stringify({
isAuthenticated: true,
sessionToken: 'nested-token',
accessToken: 'nested-access-token',
}),
nestedObject: {
token: 'object-token',
child: { refreshToken: 'object-refresh-token', safe: 'keep-me' },
},
nestedArray: [{ accessToken: 'array-access-token', safe: 'array-safe' }],
harmless: JSON.stringify({ theme: 'dark' }),
});
await mod.userScopedStorage.setItem('persist:coreState', persistedReduxBlob);
const stored = localStorage.getItem('user-123:persist:coreState');
expect(stored).not.toBeNull();
expect(stored).not.toContain('top-level-token');
expect(stored).not.toContain('nested-token');
expect(stored).not.toContain('nested-access-token');
expect(stored).not.toContain('object-token');
expect(stored).not.toContain('object-refresh-token');
expect(stored).not.toContain('array-access-token');
expect(stored).toContain('keep-me');
expect(stored).toContain('array-safe');
expect(stored).toContain('dark');
});
});
+61 -1
View File
@@ -179,6 +179,66 @@ function namespacedKey(key: string): string | null {
return `${activeUserId}:${key}`;
}
const SENSITIVE_PERSIST_KEYS = new Set(['sessionToken', 'token', 'accessToken', 'refreshToken']);
function redactSensitivePersistValue(value: unknown): unknown {
if (Array.isArray(value)) {
return value.map(redactSensitivePersistValue);
}
if (!value || typeof value !== 'object') {
return value;
}
const next: Record<string, unknown> = {};
for (const [key, child] of Object.entries(value as Record<string, unknown>)) {
if (SENSITIVE_PERSIST_KEYS.has(key)) {
continue;
}
next[key] = redactSensitivePersistValue(child);
}
return next;
}
function sanitizePersistBlob(value: string): string {
try {
const parsed = JSON.parse(value) as Record<string, unknown>;
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
return value;
}
let changed = false;
const sanitized: Record<string, unknown> = {};
for (const [key, child] of Object.entries(parsed)) {
if (SENSITIVE_PERSIST_KEYS.has(key)) {
changed = true;
continue;
}
if (typeof child === 'string') {
try {
const nested = JSON.parse(child);
const redacted = redactSensitivePersistValue(nested);
const nextChild = JSON.stringify(redacted);
sanitized[key] = nextChild;
changed ||= nextChild !== child;
continue;
} catch {
// redux-persist stores many slice fields as JSON strings, but plain
// strings are valid too; leave non-JSON strings untouched.
}
}
const redacted = redactSensitivePersistValue(child);
sanitized[key] = redacted;
changed ||= redacted !== child;
}
return changed ? JSON.stringify(sanitized) : value;
} catch {
return value;
}
}
/**
* `Storage`-shaped object compatible with redux-persist's storage contract.
* Methods return promises because redux-persist treats storage as async.
@@ -199,7 +259,7 @@ export const userScopedStorage = {
const ns = namespacedKey(key);
if (!ns) return;
try {
localStorage.setItem(ns, value);
localStorage.setItem(ns, sanitizePersistBlob(value));
} catch {
// ignore quota / unavailable
}
+33 -12
View File
@@ -278,6 +278,15 @@ fn push_failure(
));
}
fn rotated_key_log_detail(after_rotate_index: usize, total: usize) -> String {
let slot = if total == 0 {
0
} else {
after_rotate_index.saturating_sub(1) % total + 1
};
format!("slot={slot}/{total}")
}
/// Format the final bail message produced when every provider+model in the
/// chain has failed.
///
@@ -447,12 +456,15 @@ impl Provider for ReliableProvider {
// On rate-limit, try rotating API key
if rate_limited && !non_retryable_rate_limit {
if let Some(new_key) = self.rotate_key() {
if self.rotate_key().is_some() {
tracing::info!(
provider = provider_name,
error = %error_detail,
"Rate limited, rotated API key (key ending ...{})",
&new_key[new_key.len().saturating_sub(4)..]
key_slot = %rotated_key_log_detail(
self.key_index.load(Ordering::Relaxed),
self.api_keys.len()
),
"Rate limited, rotated API key"
);
}
}
@@ -579,12 +591,15 @@ impl Provider for ReliableProvider {
);
if rate_limited && !non_retryable_rate_limit {
if let Some(new_key) = self.rotate_key() {
if self.rotate_key().is_some() {
tracing::info!(
provider = provider_name,
error = %error_detail,
"Rate limited, rotated API key (key ending ...{})",
&new_key[new_key.len().saturating_sub(4)..]
key_slot = %rotated_key_log_detail(
self.key_index.load(Ordering::Relaxed),
self.api_keys.len()
),
"Rate limited, rotated API key"
);
}
}
@@ -739,12 +754,15 @@ impl Provider for ReliableProvider {
);
if rate_limited && !non_retryable_rate_limit {
if let Some(new_key) = self.rotate_key() {
if self.rotate_key().is_some() {
tracing::info!(
provider = provider_name,
error = %error_detail,
"Rate limited, rotated API key (key ending ...{})",
&new_key[new_key.len().saturating_sub(4)..]
key_slot = %rotated_key_log_detail(
self.key_index.load(Ordering::Relaxed),
self.api_keys.len()
),
"Rate limited, rotated API key"
);
}
}
@@ -864,12 +882,15 @@ impl Provider for ReliableProvider {
);
if rate_limited && !non_retryable_rate_limit {
if let Some(new_key) = self.rotate_key() {
if self.rotate_key().is_some() {
tracing::info!(
provider = provider_name,
error = %error_detail,
"Rate limited, rotated API key (key ending ...{})",
&new_key[new_key.len().saturating_sub(4)..]
key_slot = %rotated_key_log_detail(
self.key_index.load(Ordering::Relaxed),
self.api_keys.len()
),
"Rate limited, rotated API key"
);
}
}
@@ -857,6 +857,15 @@ fn failure_reason_upstream_unhealthy_wins_over_all_others() {
// configured a chain yet, so the next step is obvious without
// re-reading the docs.
#[test]
fn rotated_key_log_detail_does_not_expose_key_suffix() {
let detail = rotated_key_log_detail(2, 4);
assert_eq!(detail, "slot=2/4");
assert!(!detail.contains("sk-"));
assert!(!detail.contains("..."));
}
#[test]
fn format_failure_aggregate_prepends_user_hint_when_no_fallbacks_configured() {
let failures = vec![