Skip to content

feat(terminal): upload files into selected distro - #40

Open
mohit-kota wants to merge 3 commits into
RandomCoderOrg:mainfrom
mohit-kota:feat-add-new-terminals
Open

mohit-kota wants to merge 3 commits into
RandomCoderOrg:mainfrom
mohit-kota:feat-add-new-terminals

Conversation

@mohit-kota

@mohit-kota mohit-kota commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

  • What does this PR do?
    Adds a visible Upload file action to the active terminal toolbar. The action opens Android’s system document picker and copies one selected file into ~/Uploads inside the Linux distro associated with the terminal that initiated the upload.

    The upload runs outside the UI thread, displays progress and success/failure feedback, creates imported files with 0600 permissions, avoids overwriting existing files, and removes incomplete files after normal I/O failures.

  • Why is this change necessary?
    Users need a simple way to import scripts, PEM files, configuration files, archives, and other files already stored on their Android device. Previously this required downloading the file again or using a separate transfer mechanism.

    The initiating terminal’s root filesystem is captured before the picker opens, so switching tabs while choosing a file cannot redirect the upload into another distro.

Closes #37

Behavior and scope

Scenario Result
Tabs use the same root filesystem The uploaded file is visible from both tabs
Tabs use different distros The file remains isolated to the initiating distro
User switches tabs while the picker is open Upload still targets the initiating distro
Destination filename already exists Upload fails without overwriting the existing file
Copy fails normally The incomplete staging file is removed

This PR intentionally supports one file per upload. Folder uploads, batch uploads, exports, and a full file manager remain out of scope.

Reviewer guide

Suggested review order:

  1. TerminalFileImporter.kt — filename handling, permissions, collision behavior, and safe copy
  2. MainActivity.kt — document picker, captured distro target, and background execution
  3. TerminalPage.kt — visible upload action, progress indicator, and result message
  4. TerminalFileImporterTest.kt — isolation, cleanup, permissions, and overwrite protection

No new dependency, Android storage permission, service, or cross-distro mount is introduced.

How Has This Been Tested?

  • Automated Tests

    • ./gradlew :app:testDebugUnitTest :app:lintDebug
    • Added unit coverage for distro isolation, filename traversal, 0600 permissions, failed-copy cleanup, and overwrite protection.
  • Manual Verification

    • Samsung SM-A715F
    • Android 13 / API 33
    • Selected udroid-upload-test.sh through Android DocumentsUI
    • Confirmed the file appeared inside ~/Uploads
    • Confirmed permissions were -rw-------
    • Executed the uploaded script successfully inside Ubuntu Jammy
    • Confirmed the terminal remained usable during the flow

Screenshots

Android file picker Upload result in terminal
Android system file picker showing the selected upload test script uDroid terminal showing the uploaded script, file permissions, and successful execution

Checklist

  • The change follows the project’s existing Kotlin and Compose structure.
  • I have performed a self-review of the complete diff.
  • New non-trivial file-import behavior has focused unit coverage.
  • No explanatory comments were needed beyond the existing names and structure.
  • Documentation impact was reviewed; the destination path is documented directly in the UI.
  • Local tests and lint complete successfully.
  • No new dependency, permission, or downstream-module change is required.

Comment on lines +71 to +83
internal fun safeFileName(sourceName: String?): String {
val sanitized =
sourceName
.orEmpty()
.substringAfterLast('/')
.substringAfterLast('\\')
.map { character ->
if (character.code < 32 || character.code == 127) '_' else character
}.joinToString("")
.trim()
.take(MAX_FILE_NAME_CHARS)
return sanitized.takeUnless { it.isBlank() || it == "." || it == ".." } ?: "upload"
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any specific reason MAX_FILE_NAME_CHARS is set? I think there is no need to trim the file names

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This implementation needs rewriting. Here, the import resolves the absolute path of rootfs / then proceeds to consider home as /root. If it does exist, then fall backs to / but at the end returns ~/Uploads/$fileName?

Here, the user can be logged in as any user; the return ~/ resolves to the home path of the currently logged-in user, which is not one of the possible places where this function stores the file, so unless the user is root and has /root the return would be invalid.

Comment on lines +41 to +43
check(!Files.exists(destination, LinkOption.NOFOLLOW_LINKS)) {
"~/Uploads/$fileName already exists"
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The check for an existing file is done only before the part upload; this does not guarantee that it won't exist or will be created in between the time when files are being copied from Android to the Linux path and moved to Uploads path.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] Support file uploads to terminal sessions

2 participants