Skip to content

test(security): cover credential auto-lock session purge#860

Open
bhavyajain0810 wants to merge 1 commit into
shouri123:mainfrom
bhavyajain0810:fix/credential-auto-lock-session-purge-654
Open

test(security): cover credential auto-lock session purge#860
bhavyajain0810 wants to merge 1 commit into
shouri123:mainfrom
bhavyajain0810:fix/credential-auto-lock-session-purge-654

Conversation

@bhavyajain0810

@bhavyajain0810 bhavyajain0810 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #654

This PR adds regression coverage for credential auto-lock security behavior.

The tests verify that plaintext API credentials stored in chrome.storage.session are purged when the credential vault is locked, including both manual lock and inactivity auto-lock flows. It also verifies that credential access refreshes the inactivity timer, so the auto-lock behavior is based on actual inactivity.

This strengthens the security guarantee that decrypted API keys do not remain available in session storage after the vault is locked.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Security impact

  • Covers plaintext credential cleanup from chrome.storage.session
  • Ensures auto-lock timeout follows the same lock path as manual locking
  • Prevents stale decrypted OpenAI/ElevenLabs API keys from remaining accessible after lock

Validation

  • git diff --check → passed
  • npm test -- src/utils/credentials.test.ts → passed
  • npx tsx --test src/utils/credentials.test.ts → passed

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Summary by CodeRabbit

  • Tests
    • Added coverage for credential locking behavior, including clearing stored plaintext API keys when credentials are locked.
    • Added coverage for automatic lock timing, confirming plaintext values are removed and the unlocked state is updated after inactivity.
    • Added a test to verify that accessing credentials refreshes the inactivity timer.

@bhavyajain0810
bhavyajain0810 requested a review from shouri123 as a code owner July 8, 2026 16:29
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🚀 Thank You for Contributing to Late-Meet

Please ensure that:

  • the issue was assigned to you before opening this PR
  • the PR references the related issue
  • your changes follow repository contribution guidelines
  • the project builds successfully before submission

Unassigned, duplicate, or low-quality PRs may be closed.

Thank you for contributing 💙

@github-actions github-actions Bot added type:code Type: Code change type:test Type: Test files gssoc Official GSSoC contribution issue gssoc:approved GSSoC: PR approved and scored bug Something isn't working size/M type:design Type: Design / UI / UX (Optional bonus) type:docs Type: Documentation type:feature Type: New Feature type:testing Type: Testing (Optional bonus) labels Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

👋 Thank you @bhavyajain0810 for your contribution to Late-Meet!

✅ Verified: You are assigned to the linked issue #654.

Please review any automated suggestions or code review comments that may appear below! We will review your PR as soon as possible!


Please consider starring the repository ⭐ to show your support!

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@bhavyajain0810, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16a5f4ca-26c9-4f07-b1e7-9eb59970e3ab

📥 Commits

Reviewing files that changed from the base of the PR and between 48da75a and 500a314.

📒 Files selected for processing (1)
  • src/utils/credentials.test.ts
📝 Walkthrough

Walkthrough

Three new test cases are added to the credentials test suite validating that locking credentials or triggering inactivity auto-lock purges plaintext API keys from session storage and updates unlocked state, plus a test confirming credential access refreshes the inactivity lock timer.

Changes

Credential lock and auto-lock test coverage

Layer / File(s) Summary
Lock and auto-lock purge tests
src/utils/credentials.test.ts
Adds tests confirming lockCredentials() and a simulated inactivity timeout callback both purge plaintext credentials from chrome.storage.session and set isUnlocked() to false.
Timer refresh test
src/utils/credentials.test.ts
Adds a test overriding setTimeout/clearTimeout to verify getApiCredentials() reschedules the inactivity lock timer after unlock.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR adds regression tests for inactivity auto-lock, but it does not implement the requested 30-minute RAM cache purge behavior from #654. Add the actual inactivity lock implementation that clears plaintext credentials after 30 minutes, then keep or update the regression tests to cover it.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: security-focused tests for credential auto-lock session purging.
Out of Scope Changes check ✅ Passed The changes are limited to tests covering the credential auto-lock behavior described in the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/utils/credentials.test.ts (1)

312-342: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider verifying the auto-lock delay argument.

The test correctly verifies that getApiCredentials() triggers timer rescheduling, but it doesn't capture the delay argument passed to setTimeout. Issue #654 specifies a 30-minute inactivity timeout. Capturing and asserting the delay would strengthen the regression coverage for that requirement.

♻️ Optional: capture and assert the auto-lock delay
 test("credential access refreshes the inactivity lock timer", async () => {
   const originalSetTimeout = globalThis.setTimeout;
   const originalClearTimeout = globalThis.clearTimeout;

   let setTimeoutCalls = 0;
   let clearTimeoutCalls = 0;
+  let lastDelay: number | undefined;

-  (globalThis as any).setTimeout = () => {
+  (globalThis as any).setTimeout = (_cb: () => void, delay?: number) => {
     setTimeoutCalls += 1;
+    lastDelay = delay;
     return setTimeoutCalls;
   };

Then add an assertion after unlockCredentials:

   await unlockCredentials("test-passphrase");
   assert.equal(setTimeoutCalls, 1);
+  assert.equal(lastDelay, 30 * 60 * 1000);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/credentials.test.ts` around lines 312 - 342, The inactivity-lock
test in getApiCredentials()/unlockCredentials() verifies timer rescheduling but
not the actual timeout value. Update the test to capture the delay argument
passed to the mocked globalThis.setTimeout and assert it matches the expected
30-minute auto-lock interval, while keeping the existing checks for rescheduling
and clearTimeout behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/utils/credentials.test.ts`:
- Around line 312-342: The inactivity-lock test in
getApiCredentials()/unlockCredentials() verifies timer rescheduling but not the
actual timeout value. Update the test to capture the delay argument passed to
the mocked globalThis.setTimeout and assert it matches the expected 30-minute
auto-lock interval, while keeping the existing checks for rescheduling and
clearTimeout behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0f5abf4d-b440-49f8-94a8-2c13c4425538

📥 Commits

Reviewing files that changed from the base of the PR and between d216444 and 48da75a.

📒 Files selected for processing (1)
  • src/utils/credentials.test.ts

@bhavyajain0810
bhavyajain0810 force-pushed the fix/credential-auto-lock-session-purge-654 branch from 48da75a to 500a314 Compare July 8, 2026 16:51
@github-actions github-actions Bot removed the type:docs Type: Documentation label Jul 8, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@bhavyajain0810

bhavyajain0810 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi @shouri123,

Current validation:

  • All GitHub Actions checks are passing.
  • SonarQube Quality Gate passed.
  • CodeQL/Semgrep/TruffleHog show no new alerts.
  • Unit tests and coverage check passed in CI.

The PR #860 is now only waiting for maintainer/code owner review.

Thank you!

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been marked as stale due to inactivity.

@github-actions github-actions Bot added the stale label Jul 23, 2026
@bhavyajain0810

Copy link
Copy Markdown
Contributor Author

Hi @shouri123 ,
Please review the PR #860
Thank you!

@github-actions github-actions Bot removed the stale label Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working gssoc:approved GSSoC: PR approved and scored gssoc Official GSSoC contribution issue size/M type:code Type: Code change type:design Type: Design / UI / UX (Optional bonus) type:feature Type: New Feature type:test Type: Test files type:testing Type: Testing (Optional bonus)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Purge plaintext credential keys from RAM cache when inactivity timeout fires

1 participant