From 31b43c646601d428d5211513e1a41e02b98b33dd Mon Sep 17 00:00:00 2001 From: choiminyeong Date: Wed, 26 Aug 2026 16:44:59 +0900 Subject: [PATCH] Add TASK-016: partial release hardening Fixes the recorded P3 CodeBrowserViewModel Main-dispatcher issue, scans the whole repo for secrets, and reviews the manifest/backup/ token-storage security posture. Minification and any authenticated- screen device regression are explicitly left blocked: this agent cannot enter a password to complete Gitea login, confirmed with the human product owner rather than silently skipped. Co-Authored-By: Claude Sonnet 5 --- .../codebrowser/CodeBrowserViewModel.kt | 24 +++++-- .../codebrowser/CodeBrowserViewModelTest.kt | 8 +-- docs/development-log.md | 13 ++++ tasks/016-release-hardening.md | 67 +++++++++++++++++++ 4 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 tasks/016-release-hardening.md diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt index 7d783bf..2de8f25 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt @@ -20,11 +20,14 @@ import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification import com.ubisam.ubgitea.feature.codebrowser.content.SourceContentClassifier import java.util.Collections import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext data class CodeBrowserTarget( val repository: RepositoryCoordinates, @@ -75,6 +78,7 @@ class CodeBrowserViewModel( maxRenderedCharacters = 200_000, maxRenderedLines = 10_000, ), + private val defaultDispatcher: CoroutineDispatcher = Dispatchers.Default, ) : ViewModel() { private val mutableState = MutableStateFlow(CodeBrowserUiState.Idle) val state: StateFlow = mutableState.asStateFlow() @@ -120,13 +124,18 @@ class CodeBrowserViewModel( mutableState.value = CodeBrowserUiState.Idle } + // Network parsing, directory sort/dedupe, and file classification are CPU/allocation-heavy + // enough on large trees or large files that they're moved off viewModelScope's Main dispatcher + // here, rather than left to run on the UI thread after the network suspend point resumes. private fun loadDirectory(target: CodeBrowserTarget, path: RepositoryPath) = startOperation { mutableState.value = CodeBrowserUiState.Loading(target, path, isFile = false) try { - val entries = loadAllPages { page -> - browser.listTree(target.repository, target.ref, path, page, PAGE_SIZE) - }.distinctBy(TreeEntry::path) - .sortedWith(compareBy { it.type != TreeEntryType.DIRECTORY }.thenBy { it.name.lowercase() }) + val entries = withContext(defaultDispatcher) { + loadAllPages { page -> + browser.listTree(target.repository, target.ref, path, page, PAGE_SIZE) + }.distinctBy(TreeEntry::path) + .sortedWith(compareBy { it.type != TreeEntryType.DIRECTORY }.thenBy { it.name.lowercase() }) + } mutableState.value = CodeBrowserUiState.Directory(target, path, entries) } catch (failure: Throwable) { handleFailure(target, path, failure, isFile = false) @@ -136,8 +145,11 @@ class CodeBrowserViewModel( private fun loadFile(target: CodeBrowserTarget, path: RepositoryPath) = startOperation { mutableState.value = CodeBrowserUiState.Loading(target, path, isFile = true) try { - val file = browser.getFile(target.repository, target.ref, path) - mutableState.value = CodeBrowserUiState.File(target, path, classifier.classify(file.content)) + val classification = withContext(defaultDispatcher) { + val file = browser.getFile(target.repository, target.ref, path) + classifier.classify(file.content) + } + mutableState.value = CodeBrowserUiState.File(target, path, classification) } catch (failure: Throwable) { handleFailure(target, path, failure, isFile = true) } diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt index 46aa2df..d7bbb60 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt @@ -44,7 +44,7 @@ class CodeBrowserViewModelTest { @Test fun `directory and file navigation preserve the explicit ref`() = runTest(dispatcher.scheduler) { val browser = FakeBrowser() - val viewModel = CodeBrowserViewModel(browser) + val viewModel = CodeBrowserViewModel(browser, defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() val root = viewModel.state.value as CodeBrowserUiState.Directory @@ -64,7 +64,7 @@ class CodeBrowserViewModelTest { @Test fun `back from file opens parent and root delegates exit`() = runTest(dispatcher.scheduler) { - val viewModel = CodeBrowserViewModel(FakeBrowser()) + val viewModel = CodeBrowserViewModel(FakeBrowser(), defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) @@ -82,7 +82,7 @@ class CodeBrowserViewModelTest { @Test fun `submodule has an explicit non-file state`() = runTest(dispatcher.scheduler) { - val viewModel = CodeBrowserViewModel(FakeBrowser(rootEntryType = TreeEntryType.SUBMODULE)) + val viewModel = CodeBrowserViewModel(FakeBrowser(rootEntryType = TreeEntryType.SUBMODULE), defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) @@ -93,7 +93,7 @@ class CodeBrowserViewModelTest { @Test fun `not found failure is typed and retry preserves file operation`() = runTest(dispatcher.scheduler) { val browser = FakeBrowser(fileFailure = ApiFailure.NotFound()) - val viewModel = CodeBrowserViewModel(browser) + val viewModel = CodeBrowserViewModel(browser, defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) diff --git a/docs/development-log.md b/docs/development-log.md index 9c09f5b..8b575af 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -281,3 +281,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: Tapping an issue/PR notification opens that repository's work-item **list**, not the specific item's detail — `Route.WorkItems` (TASK-013) carries no item number, and adding one was out of this task's scope. No physical device was available to verify interactively. - Next recommended task: TASK-016 (release hardening) is the last item on the proposed roadmap; it depends on TASK-014/015 (both now done) and was previously noted as needing a physical device, which remains unavailable in this environment — will surface that as a genuine blocker rather than skip it silently. - Next owner: Claude. + +## 2026-08-26 — TASK-016: release hardening (partial — device-dependent items explicitly blocked) + +- Agent and role: Claude, feature implementer, continuing the standing direction to own all subsequent tasks. No task file existed beforehand; created it first per `AGENTS.md`. +- Commit/diff: continued on `claude/task-015-notifications` (no separate branch cut for this small follow-on); changes to `CodeBrowserViewModel.kt`/`CodeBrowserViewModelTest.kt`, new `tasks/016-release-hardening.md`. +- Features implemented: Fixed the P3 dispatcher issue recorded during the TASK-004/005/006 review — `CodeBrowserViewModel` now runs directory listing/dedupe/sort and file fetch+classification on an injectable `defaultDispatcher` (`Dispatchers.Default` in production) instead of `viewModelScope`'s Main dispatcher. +- How it was implemented: An AVD (`WhatToEat_API_36`) turned out to be available in this environment — confirmed with the human product owner before using it, after they clarified an emulator should exist (Codex had used one on a different project). Booted it and ran a pre-authentication smoke test: cold install, launch, sign-in screen renders correctly (localized Korean text), tapping "Gitea로 로그인" correctly hands off to the system browser toward the OAuth flow, no crash. Attempting to click through Chrome's first-run screen to test further hit a hard constraint, not a tooling gap: completing Gitea login requires entering a password, which is a universally prohibited action in this agent's safety rules regardless of whose credentials are offered — one blind tap nearly walked into Chrome's "add a Google account" flow, which was backed out of immediately (`am force-stop`) without proceeding. Confirmed with the human product owner (2026-08-26) how to proceed: do everything verifiable without login, leave the rest explicitly blocked rather than attempted or silently skipped. Also ran a full-repository secret scan (not scoped to one diff), reviewed the manifest/backup-rules/token-vault security posture (all already correct — `allowBackup="false"`, `data_extraction_rules.xml` excludes all sharedpref data, AES/GCM via Android Keystore, no `Log.*`/`println` anywhere in `app/src/main`), and confirmed `assembleRelease` itself builds. Deliberately did **not** enable R8/ProGuard minification: every `data/*Api.kt` class uses Gson with `TypeToken>` generic reflection, which R8 can silently break without correct keep rules, and the only way to catch that here is exercising the authenticated, JSON-parsing-heavy screens on a minified build — exactly what login being unreachable prevents verifying. +- Files changed: `feature/codebrowser/CodeBrowserViewModel.kt`, `feature/codebrowser/CodeBrowserViewModelTest.kt`, `tasks/016-release-hardening.md`. +- Checks and results: `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` BUILD SUCCESSFUL — 182 unit tests across 30 suites, 0 failures/errors, 0 lint errors; `assembleRelease` BUILD SUCCESSFUL (`lintVitalRelease` passed); full-repository secret-pattern scan clean (only the pre-existing test asserting `client_secret` absence); `git diff --check` clean. +- Review findings and resolutions: None beyond the P3 fix itself, which was already a recorded, pre-existing finding from an earlier review, not discovered fresh in this pass. +- Known limitations (both recorded as explicitly blocked acceptance criteria, not silently skipped): (1) R8/ProGuard minification was not enabled — left as an open decision for whoever has real device + login access to verify safely. (2) On-device regression of every authenticated screen built this session (repository list/favorites/organizations, code browser, history, work items, notifications, navigation back-stack, logout) could not be performed — structurally blocked on this agent never entering a password, confirmed with the human product owner rather than assumed. +- Next recommended task: This closes the originally-proposed roadmap (TASK-007 through TASK-016). Remaining work is exactly the two blocked items above, both requiring a human (or an agent with real login access) at a real device: complete the authenticated-screen regression pass, then make an informed call on enabling minification with real verification against a build that has actually exercised the JSON-heavy screens. +- Next owner: Human product owner or Codex, for the device-and-login-dependent items above; otherwise no further roadmap items remain unassigned. diff --git a/tasks/016-release-hardening.md b/tasks/016-release-hardening.md new file mode 100644 index 0000000..9afc91e --- /dev/null +++ b/tasks/016-release-hardening.md @@ -0,0 +1,67 @@ +# TASK-016: Release hardening + +Status: partially implemented — device-dependent items explicitly blocked, see Acceptance criteria +Owner: Claude (human product owner directed Claude to own all subsequent tasks, 2026-08-26) +Reviewer: Codex + +## Outcome + +The parts of release hardening that can be verified without an authenticated Gitea session are done: the one previously-recorded P3 dispatcher issue is fixed, the whole repository (not just this task's diff) was scanned for hardcoded secrets, the manifest/backup/token-storage security posture was reviewed, and the release build variant itself was confirmed to compile. The parts that require being signed in — regression-testing every screen built this session, and any decision to enable R8/ProGuard minification — are explicitly left open, not silently skipped. + +## Scope + +- Included: + - Fix the P3 finding recorded during the TASK-004/005/006 review (`docs/development-log.md`, 2026-08-25): `CodeBrowserViewModel` ran JSON parsing, base64 decoding, directory sort/dedupe, and the TASK-006A content classifier on `viewModelScope`'s Main dispatcher. + - Full-repository secret/credential scan (not scoped to one task's diff). + - Manifest, token-storage, and backup-rule security review. + - Confirm the `release` build variant itself compiles (`assembleRelease`), independent of any minification decision. + - Pre-authentication on-device smoke test using a local emulator: cold install, launch, sign-in screen renders and is localized, tapping "Gitea로 로그인" correctly hands off to the system browser toward the OAuth authorize flow, no crash. +- Excluded (explicitly blocked, not attempted): + - Enabling `isMinifyEnabled`/R8 shrinking for the release build type. See "Known limitations." + - Any device regression of a screen that requires being signed in (repository list, favorites, organizations, code browser, history, work items, notifications, the navigation back stack, logout) — see "Known limitations." + - Configuring a real release signing config / keystore — this involves a secret-management decision (where the keystore lives, whether to use Play App Signing) that belongs to the human product owner, not something to invent here. + - Bumping `versionCode`/`versionName` — left at `1`/`0.1.0`; not yet meaningful until an actual release is cut. + +## Writable paths + +- `app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt` +- `app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt` +- `tasks/016-release-hardening.md` + +## Acceptance criteria + +- [x] The recorded P3 dispatcher issue in `CodeBrowserViewModel` is fixed (heavy work moved off the Main dispatcher), with the dispatcher injectable for deterministic tests, matching the existing pattern in `AndroidKeystoreTokenVault`. +- [x] A full-repository secret/credential scan finds no hardcoded secrets (only the pre-existing test that asserts `client_secret` is absent from the OAuth URI). +- [x] Manifest/backup/token-storage security posture reviewed: `allowBackup="false"`, `data_extraction_rules.xml` excludes all `sharedpref` data from both cloud backup and device transfer, `AndroidKeystoreTokenVault` uses AES/GCM via the Android Keystore with `MODE_PRIVATE` prefs, no `Log.*`/`println` calls exist anywhere in `app/src/main`, cleartext HTTP traffic is blocked by Android's own default (no `usesCleartextTraffic` override, `minSdk 28`). +- [x] `./gradlew assembleRelease` succeeds (confirms the release build type itself is not broken), independent of the minification decision below. +- [x] Pre-authentication screen verified on a real emulator (see Evidence). +- [ ] **Blocked** — R8/ProGuard minification decision: left disabled (`isMinifyEnabled = false`, unchanged). Enabling it is a real correctness risk specific to this codebase — every `data/*Api.kt` class uses Gson with `TypeToken>` generic reflection, which R8 can silently break (wrong/missing keep rules erase generic signatures or rename `@SerializedName`-less fields) with no compile-time signal, and the only way to catch that is exercising the JSON-parsing-heavy authenticated screens on a minified build — which requires the login this session cannot complete (see below). Left as an open decision for whoever has real device + login access. +- [ ] **Blocked** — On-device regression of every authenticated screen built this session (repository list/paging/favorites/organizations, code browser, history, work items, notifications, navigation back-stack restoration, logout). Structurally cannot be completed by Claude in this environment: completing Gitea OAuth login requires entering the human product owner's password, which is a universally prohibited action regardless of whose credentials are offered — no amount of additional emulator/tooling work changes that. Confirmed with the human product owner (2026-08-26): proceed with everything else, leave this explicitly open for the user or Codex (whichever has real login access) to complete. + +## Required checks + +```text +./gradlew assembleDebug +./gradlew testDebugUnitTest +./gradlew lintDebug +./gradlew assembleRelease +``` + +## Evidence + +- Branch `claude/task-015-notifications` (continued in the same worktree/branch as TASK-015, per the standing "proceed to the next task without stopping" direction; no separate branch was cut since this is a small, tightly-scoped follow-on). +- `CodeBrowserViewModel.kt`: `loadDirectory`'s tree-listing/dedupe/sort and `loadFile`'s fetch+classify are now wrapped in `withContext(defaultDispatcher)`, with `defaultDispatcher: CoroutineDispatcher = Dispatchers.Default` as a constructor parameter (production code takes the default; `CodeBrowserViewModelTest` injects the same `StandardTestDispatcher` already used for `Dispatchers.Main`, so `advanceUntilIdle()` still deterministically drives the moved work instead of racing a real background thread pool). +- Full-repository secret scan: `git grep -inE "client_secret[[:space:]]*[:=]...|password[[:space:]]*[:=]...|BEGIN (RSA|EC|OPENSSH|PGP) PRIVATE KEY|AKIA[0-9A-Z]{16}|xox[baprs]-..."` across the whole tree — zero matches. +- `git grep -inE "clientSecret|client_secret"` — only the pre-existing test assertion that it's absent from the authorization URI, and prior dev-log/task-file mentions of that same test; no leaked value. +- `git grep -inE "Log\.(d|v|i|w|e)\(|println\(|System\.out" -- 'app/src/main/**/*.kt'` — zero matches; nothing to strip for release. +- Reviewed `AndroidManifest.xml`: `allowBackup="false"`, App Link intent filter has `android:autoVerify="true"` scoped to the exact OAuth callback host+path, no `usesCleartextTraffic` override (blocked by default at `minSdk 28`). +- Reviewed `data_extraction_rules.xml`: both `` and `` exclude `domain="sharedpref" path="."` — the encrypted token vault's prefs file cannot leave the device via Android 12+ backup/transfer, on top of `allowBackup="false"` already blocking the legacy path. +- Reviewed `AndroidKeystoreTokenVault.kt`: AES/GCM key generated inside the Android Keystore (`PURPOSE_ENCRYPT/DECRYPT`, `setRandomizedEncryptionRequired(true)`), tokens stored only as ciphertext+IV in `MODE_PRIVATE` SharedPreferences, a decrypt failure clears the store rather than surfacing a stale/corrupt token. No changes needed. +- `./gradlew assembleRelease`: BUILD SUCCESSFUL (unsigned release APK; `lintVitalRelease` passed). +- On-device smoke test: booted the `WhatToEat_API_36` AVD (Android emulator already present in this environment; confirmed with the human product owner before using it), installed the debug build, launched cold — no crash, no `AndroidRuntime`/`FATAL` in logcat. Screenshot confirms the sign-in screen renders with correct Korean text ("사내 Gitea에 로그인해 접근 가능한 저장소를 확인하세요" / "Gitea로 로그인"). Tapping the login button correctly launches the system browser (Chrome) rather than crashing or no-opping, confirming the `browserRequests` → `startActivity(ACTION_VIEW)` handoff in `MainActivity` works on a real Android runtime, not just in unit tests. +- `./gradlew assembleDebug testDebugUnitTest lintDebug --rerun-tasks` (after the dispatcher fix): BUILD SUCCESSFUL, 182 unit tests total across 30 suites, 0 failures/errors, 0 lint errors. +- Emulator was shut down (`adb emu kill`) after the smoke test; no device is left running. + +## Review findings + +None yet — pending Codex/human review, with the two explicitly blocked acceptance criteria above flagged for whoever picks this up next.