diff options
| author | Danilo M. <danix@danix.xyz> | 2026-11-03 14:44:58 +0100 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-11-03 14:44:58 +0100 |
| commit | 2b73e524d9624897f675ccc14f614fc450a7565f (patch) | |
| tree | 5f90dd4963e0aaeb2c699d86e1a2acc9dceaf366 /app/src/main/java | |
| parent | 2911783653f699e41a07bb9536460d9d0a1c3b70 (diff) | |
| download | HardenedPass-2b73e524d9624897f675ccc14f614fc450a7565f.tar.gz HardenedPass-2b73e524d9624897f675ccc14f614fc450a7565f.zip | |
fix(app): address the step 5 review and emulator findings
- A store a cancelled attempt already made for the chosen key (its
.gpg-id names exactly that key) no longer counts as occupied:
choosing the key again commits and moves on instead of refusing.
- SSH key import reads at most 100 kB whatever the provider reports,
like the PGP import, instead of reading the stream whole.
- PGPKeyManager replaces a key through a temporary file moved over it
atomically, so a failed write keeps the old key; listings only read
.key files, so a stray temporary file is never taken for a key.
- "View generated public SSH key" goes through the settings result
launcher, so the preference visibility refreshes afterwards.
- A plain start from LaunchActivity replaces the task: an open
MainActivity (e.g. one whose store vanished) is no longer left
below the new one, and one back leaves the app.
- The PGP keys screen's import FAB carries its content description
itself; M3 cleared the icon's.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Diffstat (limited to 'app/src/main/java')
6 files changed, 40 insertions, 10 deletions
diff --git a/app/src/main/java/app/passwordstore/ui/main/LaunchActivity.kt b/app/src/main/java/app/passwordstore/ui/main/LaunchActivity.kt index befd37f9..66b5cf1f 100644 --- a/app/src/main/java/app/passwordstore/ui/main/LaunchActivity.kt +++ b/app/src/main/java/app/passwordstore/ui/main/LaunchActivity.kt @@ -56,6 +56,9 @@ class LaunchActivity : AppCompatActivity() { val intentToStart = Intent(this, MainActivity::class.java) if (intent.action == ACTION_DECRYPT_PASS && isFileInRepo(filePath, repoDir)) intentToStart.putExtra(BasePGPActivity.EXTRA_FILE_PATH, filePath) + // A plain start replaces the task, so an instance already open (one whose store vanished, say) + // is not left below the new one. An entry opens on top, back returns to where the user was. + else intentToStart.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK) startActivity(intentToStart) Handler(Looper.getMainLooper()).postDelayed({ finish() }, if (noAuth) 0L else 500L) diff --git a/app/src/main/java/app/passwordstore/ui/settings/RepositorySettings.kt b/app/src/main/java/app/passwordstore/ui/settings/RepositorySettings.kt index af50b3ea..23dd2829 100644 --- a/app/src/main/java/app/passwordstore/ui/settings/RepositorySettings.kt +++ b/app/src/main/java/app/passwordstore/ui/settings/RepositorySettings.kt @@ -110,7 +110,7 @@ class RepositorySettings(private val activity: FragmentActivity) : SettingsProvi titleRes = R.string.pref_ssh_see_key_title visible = PasswordRepository.isGitRepo() && SshKey.canShowSshPublicKey onClick { - activity.startActivity(MainActivity.setupIntent(activity, SetupStart.Ssh)) + generateSshKey.launch(MainActivity.setupIntent(activity, SetupStart.Ssh)) true } } diff --git a/app/src/main/java/app/passwordstore/ui/setup/PgpKeysScreen.kt b/app/src/main/java/app/passwordstore/ui/setup/PgpKeysScreen.kt index 34e5bed6..11fd979c 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/PgpKeysScreen.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/PgpKeysScreen.kt @@ -43,6 +43,8 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp @@ -121,11 +123,13 @@ fun PgpKeysScreen( ) }, floatingActionButton = { - // M3 clears the label's semantics; the FAB takes its description from the icon. + // M3 clears the semantics of the FAB's content, label and icon alike: describe the FAB. + val label = stringResource(R.string.pref_import_pgp_key_title) ExtendedFloatingActionButton( - text = { Text(stringResource(R.string.pref_import_pgp_key_title)) }, - icon = { Icon(Icons.Filled.Add, stringResource(R.string.pref_import_pgp_key_title)) }, + text = { Text(label) }, + icon = { Icon(Icons.Filled.Add, null) }, onClick = { if (!busy) onImport() }, + modifier = Modifier.semantics { contentDescription = label }, ) }, ) { padding -> @@ -254,8 +258,8 @@ fun PgpKeysDestination( if (uri != null) viewModel.import(uri, activity) } - // ponytail: a theme or locale change in the middle cancels the commit; the next commit picks up - // the new `.gpg-id`, and choosing the key again re-creates nothing that would conflict. + // ponytail: a theme or locale change in the middle cancels the commit. Choosing the same key + // again finds the store already made for it (createdFor), commits the `.gpg-id` and moves on. fun choose(row: KeyRow) { if (viewModel.busy) return viewModel.busy = true diff --git a/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt b/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt index d270275f..8f1b52ec 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt @@ -209,12 +209,16 @@ constructor( /** * Creates the local store at [root] for [row]: a git repository and a `.gpg-id` naming the key by * its uppercase long key ID ([gpgIdText]). Returns null, or why it failed; [occupied] is the - * reason given when [root] already holds a store. + * reason given when [root] already holds a store, unless that store was made for [row] already. */ suspend fun createStore(root: File, row: KeyRow, occupied: String): String? = withContext(dispatcherProvider.io()) { - // A restored, already-cloned store must keep its .gpg-id. - if (root.isDirectory && cloneTarget(root) == CloneTarget.Occupied) return@withContext occupied + if (root.isDirectory && cloneTarget(root) == CloneTarget.Occupied) { + // Made by an earlier attempt for this key whose commit a recreation cancelled: done. + if (createdFor(root, row.keyId)) return@withContext null + // A restored, already-cloned store must keep its .gpg-id. + return@withContext occupied + } try { check(root.isDirectory || root.mkdirs()) { "Could not create $root" } PasswordRepository.createRepository(root) diff --git a/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt b/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt index ade23aa2..4e32505f 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt @@ -95,6 +95,13 @@ fun storeRecipients(text: String): List<PGPIdentifier> = fun gpgIdText(keys: List<PGPIdentifier.KeyId>): String = keys.joinToString("") { "${it.toString().uppercase(Locale.ROOT)}\n" } +/** + * Whether [dir] holds a store already made for [key]: its `.gpg-id` is exactly what creating one + * for [key] writes. Blocking: call it off the main thread. + */ +fun createdFor(dir: File, key: PGPIdentifier.KeyId): Boolean = + readGpgId(dir) == gpgIdText(listOf(key)) + private const val MAX_GPG_ID_BYTES = 64 * 1024 /** diff --git a/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt b/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt index 06cf5bc8..7c8f5ba5 100644 --- a/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt +++ b/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt @@ -16,6 +16,7 @@ import android.util.Base64 import androidx.core.content.edit import app.passwordstore.Application import app.passwordstore.R +import app.passwordstore.ui.setup.readAtMost import app.passwordstore.util.crypto.keystoreAesKey import app.passwordstore.util.crypto.requireUserAuthentication import app.passwordstore.util.crypto.seal @@ -239,7 +240,18 @@ object SshKey { val sshKeyInputStream = context.contentResolver.openInputStream(uri) ?: throw IOException(context.getString(R.string.ssh_key_does_not_exist)) - val lines = sshKeyInputStream.use { `is` -> `is`.bufferedReader().readLines() } + // Bounded whatever the provider claimed above: a larger stream is refused, not read whole. + val bytes = + sshKeyInputStream.use { readAtMost(it, 100_000) } + ?: throw IllegalArgumentException( + context.getString(R.string.ssh_key_import_error_not_an_ssh_key_message) + ) + val lines = + try { + bytes.inputStream().bufferedReader().readLines() + } finally { + bytes.fill(0) + } // Private key markers and a format sshj can read; the current key stays when this fails. if (!isSshPrivateKey(lines)) |
