Skip to content

Conversation

@criticalAY
Copy link
Contributor

Purpose / Description

This PR introduces the ProfileContextWrapper (Based on what was discussed on Discord and the structure decided), a core infrastructure component required for Multi-Profile support.

Currently, the application relies on the default Android Context for file storage and SharedPreferences. This creates a limitation where all data is stored in a single global location, making it impossible to support multiple users without data collisions or privacy leaks (e.g., sharing WebView cookies or deck_options between users).

This wrapper provides the "engine" for profile isolation by intercepting file system calls and redirecting them to profile-specific locations, without requiring a massive refactor of existing code that uses context.getFilesDir() or context.getSharedPreferences().

Fixes

Approach

  • File System Isolation: Overrides standard storage methods (getFilesDir, getCacheDir, getDatabasePath, etc.) to redirect writes to a specific subdirectory based on a static Profile ID (e.g., /data/data/pkg/p_a1b2c3d4/files).

  • SharedPreferences Namespacing: Overrides getSharedPreferences to automatically prefix preference filenames for new profiles (e.g., requesting "deck_options" transparently maps to "profile_p_a1b2_deck_options.xml").

  • Legacy Compatibility: Includes a specific check for the "default" profile ID. If active, all redirection logic is bypassed, ensuring 100% backward compatibility for existing users (their data remains in the original locations with zero migration required).

  • Initialization Safety: An init block ensures all necessary physical directories are created immediately upon wrapper instantiation to prevent "File Not Found" errors in third-party libraries.

How Has This Been Tested?

Unit tests!
i.e.
Test coverage ensures all file and preference operations stay within the injected profile directory, with correct namespacing for new profiles and preserved filenames for the default profile. It also verifies no double-prefixing occurs and that directories are created immediately on initialization.

Learning (optional, can help others)

NA

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 7, 2025
@criticalAY criticalAY force-pushed the mp/context-wrapper branch 2 times, most recently from 48d772d to a6fb080 Compare December 9, 2025 21:35
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, optional nitpicks. Cheers!

@criticalAY criticalAY force-pushed the mp/context-wrapper branch 2 times, most recently from a81bac7 to 2c0cd3a Compare December 10, 2025 00:28
@criticalAY criticalAY added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author labels Dec 10, 2025
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still an approve, just for fun:

Subject: [PATCH] fix(note-editor): Deck for new cards: 'Decide by note type'

Introduced in 2644a6d3d5b3304df1e1681a4e609af301e17f86

Related: 19650
Fixes: 19733
---
Index: AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt b/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt
--- a/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt	(revision 2c0cd3adbfe09bb3323677f47fe6b2a762f1c1cc)
+++ b/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt	(date 1765327553199)
@@ -226,7 +226,7 @@
  * Extension method to safely resolve a child file within this parent directory.
  * Prevents directory traversal attacks (e.g. "../", symlinks) by verifying canonical paths.
  *
- * @throws IllegalArgumentException If the resolved path escapes the parent directory.
+ * @throws SecurityException If the resolved path escapes the parent directory.
  */
 fun File.withFileNameSafe(childName: String): File {
     val child = File(this, childName)
@@ -235,7 +235,7 @@
         val canonicalChild = child.canonicalPath
 
         if (!canonicalChild.startsWith(canonicalParent)) {
-            throw IllegalArgumentException("Invalid path: $childName traversal attempt detected")
+            throw SecurityException("Invalid path: $childName traversal attempt detected")
         }
     } catch (e: IOException) {
         throw IllegalArgumentException("Unable to resolve canonical path for $childName", e)
Index: AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileContextWrapper.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileContextWrapper.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileContextWrapper.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileContextWrapper.kt	(revision 2c0cd3adbfe09bb3323677f47fe6b2a762f1c1cc)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileContextWrapper.kt	(date 1765327668720)
@@ -106,6 +106,40 @@
         return super.getSharedPreferences("$prefix$name", mode)
     }
 
+
+    /**
+     * Creates the directories required for the Context Wrapper.
+     *
+     * @throws SecurityException If the resolved path escapes the parent directory.
+     * @throws IOException If the directory could not be created
+     */
+    private fun requireCustomDirectories() {
+        // Default uses the base context wrapper validation
+        if (profileId.isDefault()) return
+
+        // create/validate our custom baseDir
+        if (profileBaseDir.exists()) {
+            if (!profileBaseDir.isDirectory) {
+                throw IOException("Profile root exists but is not a directory: $profileBaseDir")
+            }
+        } else {
+            if (!profileBaseDir.mkdirs()) {
+                throw IOException("Failed to create profile root directory: $profileBaseDir")
+            }
+        }
+
+        fun File.mkdirsOrFail() = if (mkdirs()) Unit
+            else throw IOException("Failed to create profile directory: $this")
+
+
+        // create the subfolders
+        this.filesDir.mkdirsOrFail()
+        this.cacheDir.mkdirsOrFail()
+        this.codeCacheDir.mkdirsOrFail()
+        this.noBackupFilesDir.mkdirsOrFail()
+        this.getDatabasePath("init").mkdirsOrFail()
+    }
+
     companion object {
         /**
          * Factory method to safely create a ProfileContextWrapper.
@@ -114,50 +148,39 @@
          *
          * @throws IllegalArgumentException If [profileBaseDir] is not inside the application's private storage.
          * @throws IOException If the profile directory structure cannot be created.
+         * @throws SecurityException If a path traversal attack occurs
          */
         @JvmStatic
         fun create(
-            base: Context,
+            context: Context,
             profileId: ProfileId,
             profileBaseDir: File,
         ): ProfileContextWrapper {
-            val appDataRoot = base.filesDir.parentFile
-            if (appDataRoot != null) {
-                val validLocation =
-                    try {
-                        profileBaseDir.canonicalPath.startsWith(appDataRoot.canonicalPath)
-                    } catch (e: IOException) {
-                        Timber.e(e, "Failed to canonicalize path: %s", profileBaseDir)
-                        false
-                    }
+
+            requirePathInFilesDir(profileBaseDir, context)
+
+            return ProfileContextWrapper(context, profileId, profileBaseDir).apply {
+                requireCustomDirectories()
+            }
+        }
+
+        /**
+         * @throws IllegalArgumentException if `profileBaseDir` is not in [Context.getFilesDir]
+         */
+        private fun requirePathInFilesDir(profileBaseDir: File, context: Context) {
+            val appDataRoot = context.filesDir.parentFile ?: return
+
+            val validLocation =
+                try {
+                    profileBaseDir.canonicalPath.startsWith(appDataRoot.canonicalPath)
+                } catch (e: IOException) {
+                    Timber.e(e, "Failed to canonicalize path: %s", profileBaseDir)
+                    false
+                }
 
-                if (!validLocation) {
-                    throw IllegalArgumentException("Profile path must be inside internal storage: $profileBaseDir")
-                }
+            if (!validLocation) {
+                throw IllegalArgumentException("Profile path must be inside internal storage: $profileBaseDir")
             }
-
-            val wrapper = ProfileContextWrapper(base, profileId, profileBaseDir)
-
-            if (!profileId.isDefault()) {
-                if (profileBaseDir.exists()) {
-                    if (!profileBaseDir.isDirectory) {
-                        throw IOException("Profile root exists but is not a directory: $profileBaseDir")
-                    }
-                } else {
-                    if (!profileBaseDir.mkdirs()) {
-                        throw IOException("Failed to create profile root directory: $profileBaseDir")
-                    }
-                }
-
-                // create the folders
-                wrapper.filesDir
-                wrapper.cacheDir
-                wrapper.codeCacheDir
-                wrapper.noBackupFilesDir
-                wrapper.getDatabasePath("init")
-            }
-
-            return wrapper
         }
     }
 }

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Second Approval Has one approval, one more approval to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants