Skip to content

feat(keychain): request storage access on auth#2377

Open
Larkooo wants to merge 2 commits intomainfrom
nas/storage-access-auth-pr
Open

feat(keychain): request storage access on auth#2377
Larkooo wants to merge 2 commits intomainfrom
nas/storage-access-auth-pr

Conversation

@Larkooo
Copy link
Contributor

@Larkooo Larkooo commented Feb 5, 2026

No description provided.

@vercel
Copy link

vercel bot commented Feb 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
controller-example-next Ready Ready Preview Feb 5, 2026 8:36pm
keychain Ready Ready Preview Feb 5, 2026 8:36pm
keychain-storybook Ready Ready Preview Feb 5, 2026 8:36pm

Request Review

@claude
Copy link
Contributor

claude bot commented Feb 5, 2026

Code Review

I found 2 issues that need attention:


🔴 High Priority: Race condition - Storage access not awaited before auth

Location: CreateController.tsx lines 506-522

The handleStorageAccessRequest callback is implemented as a fire-and-forget async operation, but all call sites proceed with onSubmit() immediately without waiting for storage access to be granted:

const handleStorageAccessRequest = useCallback(() => {
if (!isIframe()) {
return;
}
void (async () => {
try {
await requestStorageAccess();
} catch (error) {
console.error(
"[CreateController] Storage access request failed:",
error,
);
}
})();
}, []);

const handleStorageAccessRequest = useCallback(() => {
  if (!isIframe()) {
    return;
  }

  void (async () => {  // ⚠️ Fire-and-forget - nothing waits for this
    try {
      await requestStorageAccess();
    } catch (error) {
      console.error(
        "[CreateController] Storage access request failed:",
        error,
      );
    }
  })();
}, []);

All call sites follow this pattern:

onStorageAccessRequest();  // Fires async operation
onSubmit(option);          // Proceeds immediately

This creates a race condition where authentication proceeds before storage access is granted. According to the Storage Access API spec, the storage grant only takes effect after requestStorageAccess() resolves. Since the auth flow continues immediately without awaiting, it will execute before the grant is active, defeating the purpose of this feature.

Suggested fix: Make handleStorageAccessRequest async and await it before calling onSubmit:

const handleStorageAccessRequest = useCallback(async () => {
  if (!isIframe()) {
    return;
  }

  try {
    await requestStorageAccess();
  } catch (error) {
    console.error(
      "[CreateController] Storage access request failed:",
      error,
    );
  }
}, []);

// Then at call sites:
await onStorageAccessRequest();
onSubmit(option);

🟡 Low Priority: Storage access requested before guard check

Location: CreateController.tsx lines 202-208

The onStorageAccessRequest() call happens before the isDropdownOpen guard check:

ref={layoutRef}
onSubmit={(e) => {
e.preventDefault();
onStorageAccessRequest();
// Don't submit if dropdown is open
if (isDropdownOpen) {
return;
}

onSubmit={(e) => {
  e.preventDefault();
  onStorageAccessRequest();    // Called unconditionally
  // Don't submit if dropdown is open
  if (isDropdownOpen) {
    return;                    // But form doesn't submit
  }
  // ...

This means the storage access prompt may be shown to users even when the form won't actually submit (when the dropdown is open). This is a minor UX issue.

Suggested fix: Move the storage access request after the guard check:

onSubmit={(e) => {
  e.preventDefault();
  // Don't submit if dropdown is open
  if (isDropdownOpen) {
    return;
  }
  onStorageAccessRequest();
  // ...

Review Summary: Checked for bugs and CLAUDE.md compliance. Found 1 high priority race condition and 1 low priority UX issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant