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 <noreply@anthropic.com>
This commit is contained in:
parent
f14be3ac90
commit
31b43c6466
@ -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>(CodeBrowserUiState.Idle)
|
||||
val state: StateFlow<CodeBrowserUiState> = 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<TreeEntry> { 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<TreeEntry> { 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)
|
||||
}
|
||||
|
||||
@ -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())
|
||||
|
||||
@ -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<List<T>>` 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.
|
||||
|
||||
67
tasks/016-release-hardening.md
Normal file
67
tasks/016-release-hardening.md
Normal file
@ -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<List<T>>` 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 `<cloud-backup>` and `<device-transfer>` 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.
|
||||
Loading…
x
Reference in New Issue
Block a user