Prepared to ship with GOS - #2
Open
RankoR wants to merge 114 commits into
Open
Conversation
RankoR
force-pushed
the
task/prepare-to-ship
branch
from
May 6, 2026 07:07
ca1df97 to
24f878e
Compare
inthewaves
suggested changes
May 13, 2026
inthewaves
suggested changes
May 13, 2026
inthewaves
suggested changes
May 13, 2026
Member
There was a problem hiding this comment.
- Options includes a setting to change App name and this appears to be non-functional, probably should remove it
- The gallery app icon seems to change when you add it to the launcher home when Themed icons is enabled in OS Settings > Wallpaper & style > Home screen > Icons
- We could also consider a Launcher3 migration or something to migrate to the new app.grapheneos.gallery favorite, since currently the previous Gallery shortcut for the old AOSP favorite (
com.android.gallery3d/...GalleryActivityor whatever) looks like it gets removed when updating on an existing build
Comment on lines
17
to
18
Member
There was a problem hiding this comment.
Perhaps we should just use the nomaps option and remove INTERNET permission. Seems to query third-party servers for map tiles when looking at a photo with location metadata (can use GrapheneOS camera to take photo with location data)
Furthermore, tapping to see location of a photo with location metadata will actually crash with ActivityNotFoundException if there's no app to handle the map intent, and GrapheneOS doesn't include a Maps app by default
There was a problem hiding this comment.
Hello,
Not sure what the intent of the GrapheneOS team for the built-in ML models but there is a variant of the app that does not include the ML models and has nomaps as well.
Optionally, with this very recent release, there is an option to use nomaps with the ML models (pre-installed rather than downloaded from the internet) if this is the direction the team wants to take.
There were major improvements to the Vault system in the app (files inside are encrypted) in that release (and many more improvements and revamps in the past releases) so if GrapheneOS wants to keep that in then perhaps updating this fork would be beneficial.
Author
There was a problem hiding this comment.
Hi @pingu-the-penguin, thanks for the suggestion, will sync with that release.
There was a problem hiding this comment.
Hi @RankoR, thanks for your prompt reply and action!
I also wanted to note that V4.2.2 should be releasing soon (currently in nightly) and from the changelog there looks to be many security improvements such as sandboxed image decoding, per-file isolated metadata parsing and this commit.
Just something to keep in mind when it releases!
(IacobIonut01 seems to be quite active on the project lately haha)
There was a problem hiding this comment.
Bug report regardless app crash on tap on location info IacobIonut01#864
…ANAGE_EXTERNAL_STORAGE permissions
RankoR
force-pushed
the
task/prepare-to-ship
branch
from
May 14, 2026 22:01
d03af43 to
e65c1bc
Compare
The gos source set existed only to override a handful of strings, the launcher assets and two permission declarations. Keeping it in sync with upstream needed a dedicated Gradle plugin and a CI step, which is a lot of machinery for seven strings per locale. Move the overrides into src/main directly and drop the gos-string-overrides plugin, its CI check and the README section describing it.
The second VIEW/REVIEW filter matched `*/*` on the content, file and http schemes to work around JPEG-XL files not having a registered mime type. That also advertised the activity for every other type, including ones the viewer cannot render. The first filter already covers image/* and video/* on the same schemes, so drop the catch-all.
Room writes schemas to `$projectDir/schemas/` and has done since the flavor dimensions were simplified. The per-variant copies under schemas/debug, schemas/release and schemas/staging are leftovers from the old layout and are no longer regenerated.
Repositories and their data models are data-layer concerns. Domain code may depend on data, not the other way round, so move the model, repository and data-source types into feature_node.data and update every reference. Each moved interface is also co-located with its implementation, folding MediaRepositoryImpl, ExternalCropRepositoryImpl and ExternalCropIntentParserImpl into the files that declare them.
WidgetPreferences was a static object that reached for SharedPreferences and the ContentResolver on its own, so widget persistence could not be faked in a test and uri permission bookkeeping was spread across the callers. Replace it with an injected WidgetRepository that owns the SharedPreferences document and the persisted read grants, releasing the grants it no longer needs when widget data is replaced or deleted. Widget config now reports a failure to the user instead of silently finishing, and cached bitmap reads are marked @workerthread.
The observed query flows spawned a coroutine per ContentObserver callback and raced them against each other through a shared cancellation signal, so a burst of media changes could deliver a stale cursor, leak an unconsumed one, or keep a superseded query running to completion. Rewrite both flows on top of a single collectLatest pipeline: observer events are conflated and debounced, each run owns its CancellationSignal, cursors that lose the race are closed rather than emitted, and the initial stepped query only falls through to the full query when the limited page was actually full. MediaFlow gains afterId and limit so callers can walk the store in pages instead of materialising every row.
performInsertWrite inserted a visible MediaStore row and then streamed bytes into it, so a crash, a cancellation or a failed write left a truncated file visible in the gallery, and cancellation skipped cleanup entirely. Insert with IS_PENDING set, write, then clear the flag and stamp DATE_MODIFIED under NonCancellable so publication cannot be interrupted halfway. Any failure path, cancellation included, deletes the unpublished row.
The paging and cancellation behaviour only reproduces against a real MediaStore, so the unit suite cannot guard it. Build the instrumentation APK on every run and add an emulator job that executes MediaFlowPagingDeviceTest on API 36.
The JSON converter round-tripped every vector through a string, which cost both space and parse time on each read. Encode the floats into a little-endian blob with a magic number and a length prefix so a row written by a different build is rejected rather than decoded into plausible noise.
The indexer loaded every media item and every stored embedding into memory before comparing them, so the cost grew with the size of the library. Walk both sides as keyset-ordered pages and merge them, and chunk the id sets handed to SQLite so a large removal cannot exceed the bind-variable limit.
The query result type lived next to the DAO that produced it, so every consumer of a category count had to import from the data source. Move it to the model package alongside the other row types and drop the unused toCategory helper.
Classification deleted every generated mapping up front and rebuilt it as it went, so a worker that was stopped part-way left the library with fewer categories than it started with. Stage the matches under a generation id and swap them in atomically, discarding the staged rows when the run is abandoned or superseded.
The total size came from a SQL sum over the media table, which is only the search index cache and stays empty unless AI media analysis is enabled, so collections reported a size of zero for most users. Compute both the count and the size in the repository from the live media set, so the card cannot disagree with what opening the collection shows.
The field's value came from a StateFlow that was only emitted from inside a cancellable job, so two keystrokes within a frame could drop or reorder characters and the keyboard actions could act on a stale query. Let the field own its text and publish the query synchronously, keeping the flow as the channel for programmatic changes.
A dot product over mismatched arrays threw, and normalising a zero or non-finite vector produced NaNs that then poisoned every similarity score computed from it. Return a zero result in both cases instead.
Several screens rendered counts through non-plural strings, so a single item read "1 items". Route them through the existing item_count plural and drop the strings they replaced, including the translated copies.
ComponentCaller.checkContentUriPermission only accepts uris handed over at launch through the intent data, EXTRA_STREAM or the clip data, and throws for anything else. MediaStore.EXTRA_OUTPUT is a plain extra, so every external CROP request carrying an output uri threw, was swallowed by the catch, and finished with RESULT_CANCELED before any UI appeared. Check the output uri with Context.checkUriPermission instead, which accepts arbitrary uris. Both the caller and this app must hold write access: the caller check keeps us from being used as a confused deputy to overwrite media it cannot reach itself, and our own check fails fast instead of at save time. The source uri keeps checkContentUriPermission, which is the right API for intent data. Reading permission is still not enough to write the output, so a save that cannot write asks the user once through an IntentSender and resumes when the dialog returns. Tests now stub checkUriPermission and pin the regression: the write path must never route through the launch-grant API.
Media grid items resolved their own selection and metadata state, so every item recomposed on any selection change and did a linear scan of the media list per tap. Hoist the lookups into id-keyed maps built once per state change and pass the resolved values down to MediaImage. The mosaic layout is also built off the main thread, and the media mapping options are folded into a single distinct-until-changed flow so unrelated settings emissions no longer remap the whole timeline.
Nothing referenced the MediaType MediaStore lookups or the bitmap, byte array and subsampling helpers in MediaExt any more.
The database holds only derived state — the media index, metadata, category assignments and embeddings are all recomputed from MediaStore and the models. Schema version 1 has been reshaped in place (blob embeddings, staging tables), so an install carrying an older version 1 would fail its identity check on upgrade rather than only on downgrade. Fall back to a destructive rebuild in both directions instead of shipping migrations for a cache.
Object names follow class naming, not constant naming.
Sort imports the way the toolchain does — no behaviour change beyond a few named arguments and expression bodies rewritten as blocks for readability.
The screen mirrored SearchViewModel.query back into the field. That collection is asynchronous, so during fast typing the mirror lagged the keystrokes: a stale value landed mid-composition, reverted the field and then restored it. Both writes were plain string assignments carrying no selection or composition, so the ime composing region was left pointing at text that had moved — characters reordered and the composing span stranded, undeletable until the process was killed. The field now owns its text as a TextFieldState, and the view model asks it to rewrite itself only when it genuinely has different text to show — clearing, chip taps, history taps — through a one-shot queryOverrides channel. The lone space rejection moved into an InputTransformation so that path stays in sync with the ime too.
An external crop request could aim EXTRA_OUTPUT at any reachable shared media row. We hold write access to all of shared media and the caller does not, and the crop ui only ever shows the source, so a hostile caller could have us overwrite media it cannot touch itself without the user seeing the destination. Ownership is now resolved from OWNER_PACKAGE_NAME and the output is accepted only when it belongs to us or to one of the caller's packages. Neither platform permission api can answer this, for reasons the kdoc keeps recorded. This narrows the legacy contract: an output row owned by a third party, or with no recorded owner, is refused.
fallbackToDestructiveMigration would have silently discarded pinned, ignored and locked albums, groups, collections, categories and timeline settings on the first version bump after release. Only the downgrade fallback remains; schemas are exported, so every bump from here on ships a migration.
The FileUtils class had no callers and built cache paths from an unsanitized provider display name. StandaloneActivity advertised http and https, which the app cannot open without the INTERNET permission it deliberately does not hold. The maxSdkVersion storage permissions, requestLegacyExternalStorage and tools:targetApi are all inert at minSdk 36.
RankoR
force-pushed
the
task/prepare-to-ship
branch
from
July 29, 2026 21:45
6694087 to
332b1aa
Compare
inthewaves
suggested changes
Jul 30, 2026
| } | ||
|
|
||
| return isMediaStoreUri(uri) && isReachable(uri) | ||
| return isMediaStoreUri(uri) && isReachable(uri) && isOwnedByCallerOrUs(uri, caller) |
Member
There was a problem hiding this comment.
appears to regress packages/apps/AvatarPicker. AvatarPicker produces URIs that look like content://com.android.avatarpicker.tempprovider/output.png
avatars cannot be chosen due to crop activity not showing:
- Settings > System > Users > Tap on You > Set profile picture results in crop activity never showing after selecting a picture
- Settings > Safety & emergency > Emergency information > Tap on profile picture > Choose an image results in crop activity never showing after selecting a picture
better shape as AvatarPicker grants write (and read) URI permissions:
override fun canWriteContentUri(
uri: Uri,
caller: ComponentCaller
): Boolean {
if (caller.uid == Process.myUid()) {
return true
}
return if (isMediaStoreUri(uri)) {
isReachable(uri) && isOwnedByCallerOrUs(uri, caller)
} else {
hasContentUriPermission(
caller = caller,
uri = uri,
modeFlags = Intent.FLAG_GRANT_WRITE_URI_PERMISSION,
)
}
}
Would be good to have various test cases for this, e.g.
- Accept AvatarPicker shape: non-MediaStore
content://..., same URI in data, write grant present - Reject non-MediaStore
EXTRA_OUTPUTwhencheckContentUriPermission()throws or denies - Keep rejecting third-party MediaStore owner rows even if some generic URI check would pass
- Keep not opening/truncating the output during validation
Comment on lines
+51
to
+55
| * Neither platform permission api can answer this. The output uri arrives in | ||
| * [android.provider.MediaStore.EXTRA_OUTPUT], a plain extra, so it is never part of the launch | ||
| * grant set that [ComponentCaller.checkContentUriPermission] accepts — that method throws | ||
| * [IllegalArgumentException] for anything not passed via `Intent#getData`, `EXTRA_STREAM` or | ||
| * `Intent#getClipData`. Uid-based [Context.checkUriPermission] is no substitute either: it |
Member
There was a problem hiding this comment.
A counterexample to this comment is in packages/apps/AvatarPicker: https://android.googlesource.com/platform/packages/apps/AvatarPicker/+/refs/tags/android-17.0.0_r1/src/main/java/com/android/avatarpicker/domain/Utils.kt#67
fun getCropIntent(contentUri: Uri, sizeInPixels: Int) = Intent(CROP_ACTION).apply {
setDataAndType(contentUri, "image/*")
addFlags(
Intent.FLAG_GRANT_WRITE_URI_PERMISSION or Intent.FLAG_GRANT_READ_URI_PERMISSION
)
putExtra(MediaStore.EXTRA_OUTPUT, contentUri)
....
}Although EXTRA_OUTPUT itself is just a plain extra, the same URI is also put in Intent.data via setDataAndType, and the intent carries write/read grant flags. In that shape, ComponentCaller.checkContentUriPermission(outputUri, FLAG_GRANT_WRITE_URI_PERMISSION) can answer the question for the output URI.
canWriteContentUri required the output uri to be a MediaStore uri, so a caller passing an output from its own provider was refused, the parser returned null, and CropActivity finished before showing any ui. That broke Settings -> System -> Users -> You -> Set profile picture, which goes through AvatarPicker: it passes one uri from its own FileProvider as both the intent data and EXTRA_OUTPUT, with FLAG_GRANT_WRITE_URI_PERMISSION. The gate arrived in 74e3045, not with the ownership narrowing. Because that uri is in the intent data with a write grant, it is part of the launch grant set, so ComponentCaller.checkContentUriPermission can answer for it. Split the check by authority: MediaStore outputs keep the OWNER_PACKAGE_NAME rule, everything else goes through the launch grant api, which already swallows IllegalArgumentException (an output that arrives as the plain extra alone, proving nothing) and SecurityException into false. Deliberately unchanged: - MediaStore rows stay ownership-only, with no generic uri-check fallback. A third-party owned row is still refused even if the caller holds a write grant on it. - isReachable stays MediaStore-only. It is a "does the row exist" pre-check, not a security property; on the grant branch the grant answers that, and a failing write later cancels without a result. - Nothing opens the output to probe it. "w" truncates on some providers, which would destroy the target before the user confirms the crop. Also correct both KDoc claims that the output uri can never be part of the launch grant set, and note that the OutputPermissionRequired recovery path is MediaStore-specific because MediaStore.createWriteRequest cannot describe a foreign authority.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Validated with these CTS tests: