From 157c92e118893d68f63397d8726a1b13b26e6098 Mon Sep 17 00:00:00 2001 From: Devin Holland <50112339+Devin-Holland@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:20:49 -0400 Subject: [PATCH 1/2] Rebuild session.oauth when recording periodic token validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The periodic-validation path (GitHub-style non-expiring tokens) recorded the check by mutating session.oauth in place: `oauthMetadata.lastValidated = now`. On Harper v5, session.oauth is a tracked, frozen object, so that assignment throws "Cannot assign to read only property 'lastValidated'". The throw is caught and swallowed, so lastValidated never advances and the tokenValidationInterval throttle never engages — every request re-runs provider.validateToken (a live call to the provider) and re-logs the error. Rebuild session.oauth explicitly with all fields, mirroring the token refresh path. Spread cannot be used: Harper tracked objects copy nothing on `{ ...obj }`. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/sessionValidator.ts | 19 ++++++++++--- test/lib/sessionValidator.test.js | 45 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/lib/sessionValidator.ts b/src/lib/sessionValidator.ts index 3d9879d..14a57e3 100644 --- a/src/lib/sessionValidator.ts +++ b/src/lib/sessionValidator.ts @@ -84,9 +84,22 @@ export async function validateAndRefreshSession( return { valid: false, error: 'Token validation failed - token may have been revoked' }; } - // Update last validated timestamp in session - oauthMetadata.lastValidated = now; - session.oauth = oauthMetadata; + // session.oauth is a Harper tracked object: its properties are read-only and + // spread copies nothing, so rebuild it explicitly (mirrors the refresh path + // below) instead of mutating in place, which throws on the frozen record. + session.oauth = { + provider: oauthMetadata.provider, + providerConfigId: oauthMetadata.providerConfigId, + providerType: oauthMetadata.providerType, + accessToken: oauthMetadata.accessToken, + refreshToken: oauthMetadata.refreshToken, + expiresAt: oauthMetadata.expiresAt, + refreshThreshold: oauthMetadata.refreshThreshold, + scope: oauthMetadata.scope, + tokenType: oauthMetadata.tokenType, + lastRefreshed: oauthMetadata.lastRefreshed, + lastValidated: now, + }; if (typeof session.update === 'function') { await session.update(session); diff --git a/test/lib/sessionValidator.test.js b/test/lib/sessionValidator.test.js index e532815..fe06657 100644 --- a/test/lib/sessionValidator.test.js +++ b/test/lib/sessionValidator.test.js @@ -403,6 +403,51 @@ test('should perform periodic validation for non-expiring tokens', async () => { assert.ok(session.oauth.lastValidated > Date.now() - 100, 'lastValidated timestamp should be updated'); }); +test('should update lastValidated on a read-only tracked session.oauth without throwing', async () => { + let validationCalled = false; + const provider = createMockProvider({ + config: { + ...createMockProvider().config, + validateToken: async () => { + validationCalled = true; + return true; + }, + tokenValidationInterval: 1000, + }, + }); + + // Simulate a Harper GenericTrackedObject: properties are read-only (in-place + // assignment throws) and non-enumerable (spread copies nothing), as production + // session.oauth is. On the pre-fix code this made line-88's mutation throw, the + // throw was swallowed, and lastValidated never advanced -> revalidation every request. + const trackedFields = { + provider: 'github', + providerConfigId: 'github', + providerType: 'github', + accessToken: 'github_token', + refreshToken: undefined, + lastValidated: Date.now() - 2000, // 2s ago, past the interval + }; + const trackedOAuth = {}; + for (const [key, value] of Object.entries(trackedFields)) { + Object.defineProperty(trackedOAuth, key, { value, writable: false, enumerable: false, configurable: false }); + } + Object.freeze(trackedOAuth); + + const session = createMockSession({ oauth: trackedOAuth }); + + const result = await validateAndRefreshSession({ session }, provider); + + assert.strictEqual(result.valid, true); + assert.strictEqual(validationCalled, true, 'validateToken should have been called'); + assert.ok(session.oauth.lastValidated > Date.now() - 100, 'lastValidated should advance (rebuilt, not mutated)'); + // Guard the spread trap: rebuilding must copy every field explicitly. + assert.strictEqual(session.oauth.provider, 'github', 'provider preserved'); + assert.strictEqual(session.oauth.providerConfigId, 'github', 'providerConfigId preserved'); + assert.strictEqual(session.oauth.providerType, 'github', 'providerType preserved'); + assert.strictEqual(session.oauth.accessToken, 'github_token', 'accessToken preserved'); +}); + test('should skip validation when interval has not passed', async () => { let validationCalled = false; const provider = createMockProvider({ From d26818c976e609cada8feeec07aed58b3a83ecc5 Mon Sep 17 00:00:00 2001 From: Devin Holland <50112339+Devin-Holland@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:32:03 -0400 Subject: [PATCH 2/2] Assert all preserved OAuth fields in the tracked-session regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spread-trap guard only checked provider/providerConfigId/providerType/ accessToken. Give scope/tokenType/lastRefreshed real values in the tracked fixture and assert they survive the rebuild, so dropping one from the rebuild in sessionValidator.ts fails the test — TypeScript won't catch a dropped optional OAuthSessionMetadata field. Trim the test comment to intent. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/lib/sessionValidator.test.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/lib/sessionValidator.test.js b/test/lib/sessionValidator.test.js index fe06657..5435395 100644 --- a/test/lib/sessionValidator.test.js +++ b/test/lib/sessionValidator.test.js @@ -416,16 +416,18 @@ test('should update lastValidated on a read-only tracked session.oauth without t }, }); - // Simulate a Harper GenericTrackedObject: properties are read-only (in-place - // assignment throws) and non-enumerable (spread copies nothing), as production - // session.oauth is. On the pre-fix code this made line-88's mutation throw, the - // throw was swallowed, and lastValidated never advanced -> revalidation every request. + // A read-only, non-enumerable session.oauth reproduces Harper's v5 tracked object: + // in-place assignment throws and spread copies nothing. + const lastRefreshed = Date.now() - 5000; const trackedFields = { provider: 'github', providerConfigId: 'github', providerType: 'github', accessToken: 'github_token', refreshToken: undefined, + scope: 'repo read:org', + tokenType: 'bearer', + lastRefreshed, lastValidated: Date.now() - 2000, // 2s ago, past the interval }; const trackedOAuth = {}; @@ -441,11 +443,13 @@ test('should update lastValidated on a read-only tracked session.oauth without t assert.strictEqual(result.valid, true); assert.strictEqual(validationCalled, true, 'validateToken should have been called'); assert.ok(session.oauth.lastValidated > Date.now() - 100, 'lastValidated should advance (rebuilt, not mutated)'); - // Guard the spread trap: rebuilding must copy every field explicitly. assert.strictEqual(session.oauth.provider, 'github', 'provider preserved'); assert.strictEqual(session.oauth.providerConfigId, 'github', 'providerConfigId preserved'); assert.strictEqual(session.oauth.providerType, 'github', 'providerType preserved'); assert.strictEqual(session.oauth.accessToken, 'github_token', 'accessToken preserved'); + assert.strictEqual(session.oauth.scope, 'repo read:org', 'scope preserved'); + assert.strictEqual(session.oauth.tokenType, 'bearer', 'tokenType preserved'); + assert.strictEqual(session.oauth.lastRefreshed, lastRefreshed, 'lastRefreshed preserved'); }); test('should skip validation when interval has not passed', async () => {