Skip to content

Commit 7f971e5

Browse files
antonisclaude
andcommitted
fix(android): Drop SAF document authorities from getDataFromUri allowlist
Authority-only matching cannot verify that a SAF URI was granted in the current attach flow versus a previously persisted grant. The in-SDK image picker returns content://media/... on modern Android, so dropping SAF support has no impact on the FeedbackForm flow. Test inverted from allowsSafDocumentsAuthorities to rejectsSafDocumentsAuthorities. Addresses Warden review feedback on #6045. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ca3fd19 commit 7f971e5

2 files changed

Lines changed: 14 additions & 9 deletions

File tree

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,14 @@ static boolean isAllowedUri(@NotNull Uri uri, @Nullable Context ctx) {
11451145
return "file".equals(lowerScheme) && isPathUnderAppDirs(uri.getPath(), ctx);
11461146
}
11471147

1148+
// Allowlist is intentionally narrow:
1149+
// - `media` is what the in-SDK image picker flow returns.
1150+
// - the app's own FileProvider is needed when the host app exposes attachments via it.
1151+
// SAF document authorities
1152+
// (com.android.{externalstorage,providers.media,providers.downloads}.documents)
1153+
// are deliberately excluded: authority-only matching would let any caller read any document
1154+
// the app has been granted persistent SAF permission for, without verifying the URI was granted
1155+
// in the current attach flow.
11481156
@VisibleForTesting
11491157
static boolean isAllowedContentAuthority(@Nullable String authority, @Nullable Context ctx) {
11501158
if (authority == null) {
@@ -1154,11 +1162,6 @@ static boolean isAllowedContentAuthority(@Nullable String authority, @Nullable C
11541162
if ("media".equals(lower)) {
11551163
return true;
11561164
}
1157-
if ("com.android.providers.media.documents".equals(lower)
1158-
|| "com.android.externalstorage.documents".equals(lower)
1159-
|| "com.android.providers.downloads.documents".equals(lower)) {
1160-
return true;
1161-
}
11621165
if (ctx != null) {
11631166
final String pkg = ctx.getPackageName();
11641167
if (pkg != null && lower.equals(pkg.toLowerCase(Locale.ROOT) + ".fileprovider")) {

packages/core/android/src/test/java/io/sentry/react/RNSentryUriValidationTest.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,16 @@ public void allowsContentSchemeCaseInsensitive() {
7171
}
7272

7373
@Test
74-
public void allowsSafDocumentsAuthorities() {
75-
assertTrue(
74+
public void rejectsSafDocumentsAuthorities() {
75+
// SAF authorities are excluded from the allowlist: authority-only matching cannot verify
76+
// that the URI was granted in the current attach flow versus a previously persisted grant.
77+
assertFalse(
7678
RNSentryModuleImpl.isAllowedUri(
7779
mockUri("content", "com.android.providers.media.documents", "/doc"), ctx));
78-
assertTrue(
80+
assertFalse(
7981
RNSentryModuleImpl.isAllowedUri(
8082
mockUri("content", "com.android.externalstorage.documents", "/doc"), ctx));
81-
assertTrue(
83+
assertFalse(
8284
RNSentryModuleImpl.isAllowedUri(
8385
mockUri("content", "com.android.providers.downloads.documents", "/doc"), ctx));
8486
}

0 commit comments

Comments
 (0)