Skip to content

fix(macos): store desktop secrets in Keychain - #430

Open
veryCrunchy wants to merge 11 commits into
mainfrom
fix/macos-keychain-secret-store
Open

fix(macos): store desktop secrets in Keychain#430
veryCrunchy wants to merge 11 commits into
mainfrom
fix/macos-keychain-secret-store

Conversation

@veryCrunchy

Copy link
Copy Markdown
Member

What changed

  • add a native Security.framework-backed macOS Keychain secret store
  • store desktop login credentials and local Deck draft keys without requiring Linux Secret Service tools
  • handle add, update, load, clear, denied or locked Keychain access, and concurrent-add races
  • use credential-free service and account identifiers in native calls and diagnostics

Validation

  • focused DesktopSecretStoreTest with a fake native API
  • ./gradlew --no-daemon --max-workers=1 :ui:createDistributable
  • bash tools/check-kotlin-architecture.sh
  • bash tools/check-repository.sh

Limits

The implementation compiled and passed deterministic tests on Linux. A live macOS Keychain round trip and macOS package build are still required before removing the documented platform limitation.

Advances #335

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 18:08 Destroyed
@obiente-cloud

obiente-cloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

Obiente preview

NC Native · 962e6b8d4f33 · Ready

Open preview

View in Obiente

Obiente updates this comment as the preview changes.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T22:14:48.080281Z 962e6b8 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@gitguardian

gitguardian Bot commented Sep 1, 2026

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 18:10 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 18:13 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 94066a3579

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
Comment thread changes/unreleased/335-macos-keychain-secret-store.md Outdated
@veryCrunchy
veryCrunchy force-pushed the fix/macos-keychain-secret-store branch from a799b6b to 89caf2d Compare September 1, 2026 18:18
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 18:18 Destroyed
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 89caf2dcdf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 18:47 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 18:51 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74f6daa413

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
Comment thread ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt Outdated
Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 19:32 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 777213be6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 19:47 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 323f801a64

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 20:20 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 20:25 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 121797e689

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 20:59 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16380e7ef1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt Outdated
Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 21:37 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #430 / NC Native September 1, 2026 21:42 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e246a78882

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
Comment thread ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt Outdated
Comment thread website/public/screenshots/capture-manifest.json Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 962e6b8d4f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +3857 to +3858
if (failure is DesktopSecretDeletionRecoveryUnavailableException ||
failure is DesktopSecretLegacyCleanupUnavailableException) throw failure

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Queue legacy cleanup instead of aborting local sign-out

When secret-tool is absent, save() deliberately accepts a durable AdoptedPendingLegacyCleanup state, so every subsequent sign-out reaches this throw even on a fresh macOS profile. Fresh evidence beyond the earlier cleanup finding is that AccountSettingsScreen.kt:156-157 revokes the server session before calling clearSession(); this exception then preserves the in-memory session and preferences, leaving the UI logged into already-revoked credentials while every retry fails on the same missing provider. Persist the legacy target for later cleanup and allow local sign-out to finish instead of throwing after remote revocation.

AGENTS.md reference: AGENTS.md:L457-L459

Useful? React with 👍 / 👎.

if (session == null) {
var sessionLoadAttempt by remember { mutableStateOf(0) }
val sessionLoad = remember(services, sessionLoadAttempt) {
loadNextcloudSessionSafely(services::loadSession)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Lower the shrunken Kotlin file-size baseline

This change reduces NextcloudNativeApp.kt from 12,436 to 12,435 lines, but tools/kotlin-file-size-baseline.txt still permits 12,436. Leaving the old allowance lets a later edit silently restore the removed line, contrary to the repository's explicit requirement to lower an oversized file's baseline whenever it shrinks; update the entry to 12,435 in this change.

AGENTS.md reference: AGENTS.md:L214-L219

Useful? React with 👍 / 👎.

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