feat(kotlin-sdk): implement secure native library downloading with SH…#416
feat(kotlin-sdk): implement secure native library downloading with SH…#416Abhinav-kodes wants to merge 2 commits intoRunanywhereAI:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a shared Gradle build-logic module providing a native-downloader plugin and a DownloadAndVerifyNativeLibTask (with timeouts and SHA-256 verification). Applies the plugin to llamacpp and onnx modules, replacing inline download/extract logic with per-ABI tasks and an aggregate downloadJniLibs task; includes the composite build. Changes
Sequence Diagram(s)sequenceDiagram
participant Gradle as Gradle Build
participant Task as DownloadAndVerifyNativeLibTask
participant HTTP as HTTP Server
participant ZIP as Zip Extractor
participant FS as File System
Gradle->>Task: Execute per-ABI download task
Task->>HTTP: GET artifact.zip (with connect/read timeouts)
HTTP-->>Task: Streamed bytes
Task->>Task: Stream to temp file + compute SHA-256
Task->>HTTP: GET artifact.zip.sha256
HTTP-->>Task: Return expected checksum
Task->>Task: Compare computed vs expected
alt Match
Task->>ZIP: Open ZIP and filter allowed .so files
ZIP->>FS: Write selected .so files to jniLibs/<abi>
Task-->>Gradle: Task succeeds
else Mismatch
Task->>FS: Delete temp file
Task-->>Gradle: Fail with checksum error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 1c54218 in 30 seconds. Click for details.
- Reviewed
437lines of code in6files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_jGTPZetC2ybYRN1C
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt (1)
114-118: EmptyNativeLibraryDownloadPlugin.apply()body — detektEmptyFunctionBlockwarning.The plugin class exists solely for plugin ID registration; the empty
applyis intentional but triggers a detekt warning. Add a comment or a@Suppressto signal intent:✨ Proposed fix
class NativeLibraryDownloadPlugin : Plugin<Project> { - override fun apply(project: Project) { - - } + // Task type (DownloadAndVerifyNativeLibTask) is registered directly by consumers; + // no additional project configuration is required here. + `@Suppress`("EmptyFunctionBlock") + override fun apply(project: Project) = Unit }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt` around lines 114 - 118, The empty apply implementation in NativeLibraryDownloadPlugin is intentional but triggers detekt's EmptyFunctionBlock; to fix, either add a single-line comment inside apply like "// intentionally empty: plugin ID registration only" or annotate the class or method with `@Suppress`("EmptyFunctionBlock") so detekt knows this is deliberate; update NativeLibraryDownloadPlugin.apply (or the NativeLibraryDownloadPlugin class) accordingly to silence the warning while preserving the empty body.sdk/runanywhere-kotlin/build-logic/build.gradle.kts (1)
20-22: Remove emptydependenciesblock.It adds visual noise with no effect.
✨ Proposed cleanup
-dependencies { - -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/runanywhere-kotlin/build-logic/build.gradle.kts` around lines 20 - 22, Remove the empty Gradle DSL block named "dependencies" from build.gradle.kts since it has no effect; locate the empty dependencies { } declaration in the file (the dependencies block shown in the diff) and delete it to reduce visual noise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt`:
- Around line 62-66: The download flow in NativeLibraryDownloadPlugin opens an
HttpURLConnection but doesn't verify the HTTP response code, so non-2xx
responses produce a generic IOException; update the code after
connection.connect() (or immediately after opening the connection) to call
connection.responseCode (and responseMessage), and if the code is not in
200..299 throw a new IOException with a clear message that includes the URL,
responseCode and responseMessage (optionally include the error stream body)
before attempting to read connection.inputStream; this makes failures like 404
explicit and easier to debug.
- Around line 83-109: The checksum check currently embeds tempFile.delete() as a
side effect in the check { } message lambda and tempFile is only deleted after
fsOps.copy, which leaks the temp file if extraction throws; remove the
tempFile.delete() call from the check message and make the code robust by
wrapping the extraction and any subsequent work in a try/finally where
tempFile.delete() is called in the finally block (so tempFile is always
cleaned). Specifically, change the check(...) to only produce the error string
(use calculatedHash and expectedHash) and surround the fsOps.copy { ... } /
logger.lifecycle(...) block with try { ... } finally { tempFile.delete() } so
cleanup always runs; reference symbols: check, calculatedHash, expectedHash,
fsOps.copy, logger.lifecycle, tempFile, destDir, validFiles, RelativePath.
- Line 56: Replace the unprotected URL(shaUrl).readText() call used to compute
expectedHash with an HttpURLConnection-based fetch that sets connectTimeout to
30_000 and readTimeout to 120_000, reads the response body (respecting charset),
trims and splits on whitespace to get the first token; ensure you open the
connection from URL(shaUrl).openConnection() as HttpURLConnection, call
connect(), read the stream into a String, and disconnect/close streams when done
so the SHA256 fetch uses the same timeouts as the main download (reference
expectedHash variable and the same timeout values used in the main download
logic).
In `@sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts`:
- Line 163: The build config lists targetAbis = listOf("arm64-v8a",
"armeabi-v7a", "x86_64") but ndk { abiFilters } only packages "arm64-v8a" and
"x86_64", causing unnecessary downloads of armeabi-v7a; fix by either adding
"armeabi-v7a" to the abiFilters entry so it matches targetAbis or remove
"armeabi-v7a" from targetAbis in both runanywhere-core-llamacpp and
runanywhere-core-onnx so the download list aligns with the packaged ABIs (update
the targetAbis and/or abiFilters usages accordingly).
In `@sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts`:
- Line 171: targetAbis contains "armeabi-v7a" but abiFilters does not, causing
CI to build an extra ABI and waste bandwidth; update the configuration so
targetAbis and abiFilters match by either removing "armeabi-v7a" from the
targetAbis list or adding "armeabi-v7a" to the abiFilters definition so both
variables (targetAbis and abiFilters) contain the same ABI entries for the ONNX
bundle.
---
Nitpick comments:
In `@sdk/runanywhere-kotlin/build-logic/build.gradle.kts`:
- Around line 20-22: Remove the empty Gradle DSL block named "dependencies" from
build.gradle.kts since it has no effect; locate the empty dependencies { }
declaration in the file (the dependencies block shown in the diff) and delete it
to reduce visual noise.
In
`@sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt`:
- Around line 114-118: The empty apply implementation in
NativeLibraryDownloadPlugin is intentional but triggers detekt's
EmptyFunctionBlock; to fix, either add a single-line comment inside apply like
"// intentionally empty: plugin ID registration only" or annotate the class or
method with `@Suppress`("EmptyFunctionBlock") so detekt knows this is deliberate;
update NativeLibraryDownloadPlugin.apply (or the NativeLibraryDownloadPlugin
class) accordingly to silence the warning while preserving the empty body.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
sdk/runanywhere-kotlin/build-logic/build.gradle.ktssdk/runanywhere-kotlin/build-logic/settings.gradle.ktssdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
| logger.lifecycle("Fetching expected SHA256 from $shaUrl...") | ||
|
|
||
| // Grab the first sequence of non-whitespace characters (the hash itself) | ||
| val expectedHash = URL(shaUrl).readText().trim().split("\\s+".toRegex()).first() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "NativeLibraryDownloadPlugin.kt" | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 157
🏁 Script executed:
# Get the file size first to ensure we can read it
wc -l sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.ktRepository: RunanywhereAI/runanywhere-sdks
Length of output: 159
🏁 Script executed:
# Read the entire file with line numbers
cat -n sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.ktRepository: RunanywhereAI/runanywhere-sdks
Length of output: 5252
🏁 Script executed:
# Search for PR `#210` or issue references in git history
git log --oneline --all --grep="210" | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 176
🏁 Script executed:
# Search for any related commits mentioning timeout, URL.readText, or similar
git log --oneline --all --grep="timeout" | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 867
🏁 Script executed:
# Check for issue tracker references in recent commits
git log --oneline -30 | grep -i "timeout\|hash\|download\|sha"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 176
🏁 Script executed:
# Show the commit details for PR `#210`
git show 1c542182 --stat | head -50Repository: RunanywhereAI/runanywhere-sdks
Length of output: 3059
🏁 Script executed:
# Check the diff for the specific file to understand the intent
git show 1c542182:sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt | head -100Repository: RunanywhereAI/runanywhere-sdks
Length of output: 4011
Apply HttpURLConnection with connection and read timeouts to SHA256 fetch (line 56) — currently unprotected.
Line 56 uses URL(shaUrl).readText(), which lacks timeout configuration and contradicts the explicit goal of PR #210 ("implement secure native library downloading with SHA256 and timeouts"). The main download (lines 63–65) correctly enforces 30s connection and 120s read timeouts via HttpURLConnection, but the SHA256 fetch remains vulnerable to indefinite hangs from stalled servers.
Proposed fix — fetch SHA256 via HttpURLConnection with matching timeouts
- val expectedHash = URL(shaUrl).readText().trim().split("\\s+".toRegex()).first()
+ var shaConnection: HttpURLConnection? = null
+ val expectedHash: String
+ try {
+ shaConnection = URL(shaUrl).openConnection() as HttpURLConnection
+ shaConnection.connectTimeout = 30_000
+ shaConnection.readTimeout = 30_000
+ expectedHash = shaConnection.inputStream.bufferedReader()
+ .use { it.readText() }
+ .trim()
+ .split("\\s+".toRegex())
+ .first()
+ } finally {
+ shaConnection?.disconnect()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val expectedHash = URL(shaUrl).readText().trim().split("\\s+".toRegex()).first() | |
| var shaConnection: HttpURLConnection? = null | |
| val expectedHash: String | |
| try { | |
| shaConnection = URL(shaUrl).openConnection() as HttpURLConnection | |
| shaConnection.connectTimeout = 30_000 | |
| shaConnection.readTimeout = 30_000 | |
| expectedHash = shaConnection.inputStream.bufferedReader() | |
| .use { it.readText() } | |
| .trim() | |
| .split("\\s+".toRegex()) | |
| .first() | |
| } finally { | |
| shaConnection?.disconnect() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt`
at line 56, Replace the unprotected URL(shaUrl).readText() call used to compute
expectedHash with an HttpURLConnection-based fetch that sets connectTimeout to
30_000 and readTimeout to 120_000, reads the response body (respecting charset),
trims and splits on whitespace to get the first token; ensure you open the
connection from URL(shaUrl).openConnection() as HttpURLConnection, call
connect(), read the stream into a String, and disconnect/close streams when done
so the SHA256 fetch uses the same timeouts as the main download (reference
expectedHash variable and the same timeout values used in the main download
logic).
sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Show resolved
Hide resolved
sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Outdated
Show resolved
Hide resolved
|
|
||
| // Download LlamaCPP backend libs from GitHub releases (testLocal=false) | ||
| val releaseBaseUrl = "https://github.com/RunanywhereAI/runanywhere-sdks/releases/download/v$nativeLibVersion" | ||
| val targetAbis = listOf("arm64-v8a", "armeabi-v7a", "x86_64") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm abiFilters vs targetAbis across both affected modules
rg -n "abiFilters|targetAbis" \
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts \
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsRepository: RunanywhereAI/runanywhere-sdks
Length of output: 853
targetAbis includes armeabi-v7a but abiFilters does not — unnecessary download in both modules.
Both runanywhere-core-llamacpp and runanywhere-core-onnx have the same mismatch: targetAbis lists arm64-v8a, armeabi-v7a, and x86_64 (3 ABIs), but ndk { abiFilters } only packages arm64-v8a and x86_64. The armeabi-v7a native library (~34 MB per download) is fetched on every non-testLocal build but never included in the output, wasting CI bandwidth. Either add armeabi-v7a to abiFilters to align with the download list, or remove it from targetAbis if it is not required.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts` at
line 163, The build config lists targetAbis = listOf("arm64-v8a", "armeabi-v7a",
"x86_64") but ndk { abiFilters } only packages "arm64-v8a" and "x86_64", causing
unnecessary downloads of armeabi-v7a; fix by either adding "armeabi-v7a" to the
abiFilters entry so it matches targetAbis or remove "armeabi-v7a" from
targetAbis in both runanywhere-core-llamacpp and runanywhere-core-onnx so the
download list aligns with the packaged ABIs (update the targetAbis and/or
abiFilters usages accordingly).
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts
Outdated
Show resolved
Hide resolved
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Flatten the directory structure | ||
| relativePath = RelativePath(true, name) | ||
| } else { | ||
| exclude() |
There was a problem hiding this comment.
exclude() is not valid inside eachFile. To skip files, set relativePath to null or omit the file
| exclude() | |
| exclude() |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Line: 103
Comment:
`exclude()` is not valid inside `eachFile`. To skip files, set `relativePath` to `null` or omit the file
```suggestion
exclude()
```
How can I resolve this? If you propose a fix, please make it concise.| // Grab the first sequence of non-whitespace characters (the hash itself) | ||
| val expectedHash = URL(shaUrl).readText().trim().split("\\s+".toRegex()).first() |
There was a problem hiding this comment.
No error handling for the SHA256 file download. As noted in the PR description, this will throw FileNotFoundException when .sha256 files don't exist in releases. Wrap in try-catch and provide a clear error message about missing checksums
| // Grab the first sequence of non-whitespace characters (the hash itself) | |
| val expectedHash = URL(shaUrl).readText().trim().split("\\s+".toRegex()).first() | |
| // Grab the first sequence of non-whitespace characters (the hash itself) | |
| val expectedHash = try { | |
| URL(shaUrl).readText().trim().split("\\s+".toRegex()).first() | |
| } catch (e: Exception) { | |
| throw IllegalStateException("Failed to fetch SHA256 checksum from $shaUrl. Ensure release assets include .sha256 files.", e) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Line: 55-56
Comment:
No error handling for the SHA256 file download. As noted in the PR description, this will throw `FileNotFoundException` when `.sha256` files don't exist in releases. Wrap in try-catch and provide a clear error message about missing checksums
```suggestion
// Grab the first sequence of non-whitespace characters (the hash itself)
val expectedHash = try {
URL(shaUrl).readText().trim().split("\\s+".toRegex()).first()
} catch (e: Exception) {
throw IllegalStateException("Failed to fetch SHA256 checksum from $shaUrl. Ensure release assets include .sha256 files.", e)
}
```
How can I resolve this? If you propose a fix, please make it concise.| logger.lifecycle("Fetching expected SHA256 from $shaUrl...") | ||
|
|
||
| // Grab the first sequence of non-whitespace characters (the hash itself) | ||
| val expectedHash = URL(shaUrl).readText().trim().split("\\s+".toRegex()).first() |
There was a problem hiding this comment.
SHA256 URL fetch has no timeout configured. If the checksum file fetch hangs, the entire build will stall indefinitely. Set connection and read timeouts similar to lines 64-65
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Line: 56
Comment:
SHA256 URL fetch has no timeout configured. If the checksum file fetch hangs, the entire build will stall indefinitely. Set connection and read timeouts similar to lines 64-65
How can I resolve this? If you propose a fix, please make it concise.| check(calculatedHash.equals(expectedHash, ignoreCase = true)) { | ||
| tempFile.delete() // Nuke the bad file |
There was a problem hiding this comment.
Temp file cleanup happens inside the error message construction, which only executes if the check fails. If verification succeeds but extraction fails later, tempFile won't be deleted until line 109. Consider using try-finally or tempFile.deleteOnExit() for guaranteed cleanup
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Line: 85-86
Comment:
Temp file cleanup happens inside the error message construction, which only executes if the check fails. If verification succeeds but extraction fails later, `tempFile` won't be deleted until line 109. Consider using try-finally or `tempFile.deleteOnExit()` for guaranteed cleanup
How can I resolve this? If you propose a fix, please make it concise.| include("**/*.so") | ||
| eachFile { | ||
| if (validFiles.contains(name)) { | ||
| // Flatten the directory structure | ||
| relativePath = RelativePath(true, name) | ||
| } else { | ||
| exclude() | ||
| } | ||
| } |
There was a problem hiding this comment.
File filtering logic will include all .so files first (line 97), then try to filter in eachFile. Files not in validFiles list will still be included because exclude() inside eachFile doesn't work as expected. Use include with explicit patterns or filter differently
| include("**/*.so") | |
| eachFile { | |
| if (validFiles.contains(name)) { | |
| // Flatten the directory structure | |
| relativePath = RelativePath(true, name) | |
| } else { | |
| exclude() | |
| } | |
| } | |
| include { element -> | |
| element.isDirectory || (element.name.endsWith(".so") && validFiles.contains(element.name)) | |
| } | |
| eachFile { | |
| // Flatten the directory structure | |
| relativePath = RelativePath(true, name) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/build-logic/src/main/kotlin/NativeLibraryDownloadPlugin.kt
Line: 97-105
Comment:
File filtering logic will include all `.so` files first (line 97), then try to filter in `eachFile`. Files not in `validFiles` list will still be included because `exclude()` inside `eachFile` doesn't work as expected. Use `include` with explicit patterns or filter differently
```suggestion
include { element ->
element.isDirectory || (element.name.endsWith(".so") && validFiles.contains(element.name))
}
eachFile {
// Flatten the directory structure
relativePath = RelativePath(true, name)
}
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| // Download LlamaCPP backend libs from GitHub releases (testLocal=false) | ||
| val releaseBaseUrl = "https://github.com/RunanywhereAI/runanywhere-sdks/releases/download/v$nativeLibVersion" | ||
| val targetAbis = listOf("arm64-v8a", "armeabi-v7a", "x86_64") |
There was a problem hiding this comment.
targetAbis includes "armeabi-v7a" but the ndk.abiFilters on line 127 only includes "arm64-v8a" and "x86_64". This mismatch means the armeabi-v7a libraries will be downloaded but never packaged into the AAR
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts
Line: 163
Comment:
`targetAbis` includes `"armeabi-v7a"` but the `ndk.abiFilters` on line 127 only includes `"arm64-v8a"` and `"x86_64"`. This mismatch means the `armeabi-v7a` libraries will be downloaded but never packaged into the AAR
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| // Download ONNX backend libs from GitHub releases (testLocal=false) | ||
| val releaseBaseUrl = "https://github.com/RunanywhereAI/runanywhere-sdks/releases/download/v$nativeLibVersion" | ||
| val targetAbis = listOf("arm64-v8a", "armeabi-v7a", "x86_64") |
There was a problem hiding this comment.
targetAbis includes "armeabi-v7a" but the ndk.abiFilters on line 134 only includes "arm64-v8a" and "x86_64". This mismatch means the armeabi-v7a libraries will be downloaded but never packaged into the AAR
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts
Line: 171
Comment:
`targetAbis` includes `"armeabi-v7a"` but the `ndk.abiFilters` on line 134 only includes `"arm64-v8a"` and `"x86_64"`. This mismatch means the `armeabi-v7a` libraries will be downloaded but never packaged into the AAR
How can I resolve this? If you propose a fix, please make it concise.…wnloader Addresses review feedback from CodeRabbit and Greptile to ensure the native library download pipeline is fully secure, reliable, and efficient. Key fixes: - Added HttpURLConnection timeouts (30s) to the SHA256 checksum fetch to prevent CI hangs. - Added strict HTTP response code validation (200-299) for both checksum and binary downloads, providing clear error messages on 404s. - Fixed a potential resource leak by wrapping archive extraction in a try/finally block to guarantee temp zip deletion. - Corrected archive file filtering by using dynamic `include()` rules instead of the buggy `exclude()` inside Gradle's `eachFile` block. - Removed unused `armeabi-v7a` from `targetAbis` in both llamacpp and onnx modules to match `ndk.abiFilters`, saving ~60MB of wasted downloads per CI run.
|
@sanchitmonga22 please have a look What it does: Replaces the old insecure native library downloader with one that verifies SHA256 checksums so nobody can tamper with the downloaded files. The issue: The .sha256 files don't exist on GitHub Releases yet, so it'll fail on first run. The release pipeline needs to be updated to generate them first. |
|
Hey @sanchitmonga22, just a gentle ping on this! It's been a few days since the last review round. All the critical issues flagged by CodeRabbit and Greptile have been fixed in the latest commit.The only blocker now is on the release pipeline side: the .sha256 files need to be generated and uploaded to the GitHub Release assets before this can run end-to-end. Once that's in place, this PR is ready to merge.Let me know if there's anything else needed from my side! |
2 participants
Description
Fixes #210.
This PR secures the build-time native library download pipeline in the Kotlin SDK against supply-chain vulnerabilities and network hangs.
Key Changes:
build-logicconvention plugin to centralize the shared download logic via a customDownloadAndVerifyNativeLibTask.ant.withGroovyBuildercalls inrunanywhere-core-llamacppandrunanywhere-core-onnx.HttpURLConnectionwith strict timeouts (30s connect / 120s read). The task now calculates the SHA256 hash during the stream and validates it against the expected.sha256release asset before extracting the binaries.doLastblocks. By injectingFileSystemOperationsandArchiveOperationsinto aDefaultTaskwith@Inputand@OutputDirectoryannotations, Gradle can now safely cache these tasks, execute the ABI downloads in parallel, and fully support the Configuration Cache..sofiles, ignoring unnecessaryMETA-INForLICENSEfiles from the release archives.Type of Change
Testing
./gradlew buildEnvironmentand local task execution)Platform-Specific Testing (check all that apply)
(Note: These changes only affect the Gradle build system for the Kotlin SDK, not the runtime code)
Swift SDK / iOS Sample:
Kotlin SDK / Android Sample:
Flutter SDK / Flutter Sample:
React Native SDK / React Native Sample:
Playground:
Web SDK / Web Sample:
Labels
Please add the appropriate label(s):
SDKs:
Swift SDK- Changes to Swift SDK (sdk/runanywhere-swift)Kotlin SDK- Changes to Kotlin SDK (sdk/runanywhere-kotlin)Flutter SDK- Changes to Flutter SDK (sdk/runanywhere-flutter)React Native SDK- Changes to React Native SDK (sdk/runanywhere-react-native)Web SDK- Changes to Web SDK (sdk/runanywhere-web)Commons- Changes to shared native code (sdk/runanywhere-commons)Sample Apps:
iOS Sample- Changes to iOS example app (examples/ios)Android Sample- Changes to Android example app (examples/android)Flutter Sample- Changes to Flutter example app (examples/flutter)React Native Sample- Changes to React Native example app (examples/react-native)Web Sample- Changes to Web example app (examples/web)Checklist
Screenshots
N/A - Gradle Build System changes only.
Important
Securely download native libraries in Kotlin SDK using a new plugin with checksum verification, replacing unsafe methods.
build-logicconvention plugin withDownloadAndVerifyNativeLibTaskfor secure native library downloading.HttpURLConnectionwith timeouts and SHA256 checksum verification.runanywhere-core-llamacppandrunanywhere-core-onnxto use the new plugin for downloading native libraries.ant.withGroovyBuildercalls.runanywhere-core-llamacppandrunanywhere-core-onnx.This description was created by
for 1c54218. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Refactor
Chores
Greptile Summary
Replaced unsafe
ant.withGroovyBuilderdownload tasks with a centralized Gradle convention plugin that implements SHA256 checksum verification and connection timeouts. The newDownloadAndVerifyNativeLibTaskdownloads native libraries from GitHub releases, verifies their integrity, and extracts only permitted.sofiles.Key improvements:
.sofilesCritical issues found:
exclude()insideeachFiledoesn't work, potentially extracting unwanted filesarmeabi-v7alibraries that aren't included inndk.abiFiltersConfidence Score: 2/5
exclude()usage insideeachFile, the missing error handling will cause cryptic failures when SHA256 files don't exist (as acknowledged in the PR description), and the SHA256 fetch has no timeout protection. Additionally, there's an ABI configuration mismatch that wastes CI time downloading unused librariesNativeLibraryDownloadPlugin.kt- the file filtering and error handling logic must be fixed before this can work correctlyImportant Files Changed
exclude()insideeachFile, and no timeout on checksum URL fetchtargetAbisincludesarmeabi-v7awhich is not inndk.abiFilters, causing unnecessary downloads of libraries that won't be packagedtargetAbisincludesarmeabi-v7awhich is not inndk.abiFilters, causing unnecessary downloads of libraries that won't be packagedFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Start[Gradle Build Triggered] --> CheckLocal{testLocal flag?} CheckLocal -->|true| Skip[Skip download, use local libs] CheckLocal -->|false| ParallelDownload[Parallel ABI Downloads] ParallelDownload --> Arm64[downloadJniLibs_arm64_v8a] ParallelDownload --> ArmV7[downloadJniLibs_armeabi_v7a] ParallelDownload --> X64[downloadJniLibs_x86_64] Arm64 --> FetchSHA1[Fetch SHA256 from GitHub] ArmV7 --> FetchSHA2[Fetch SHA256 from GitHub] X64 --> FetchSHA3[Fetch SHA256 from GitHub] FetchSHA1 --> Download1[Download ZIP with timeouts] FetchSHA2 --> Download2[Download ZIP with timeouts] FetchSHA3 --> Download3[Download ZIP with timeouts] Download1 --> Verify1[Stream & calculate SHA256] Download2 --> Verify2[Stream & calculate SHA256] Download3 --> Verify3[Stream & calculate SHA256] Verify1 --> Check1{Checksum match?} Verify2 --> Check2{Checksum match?} Verify3 --> Check3{Checksum match?} Check1 -->|no| Fail1[Delete ZIP, throw error] Check2 -->|no| Fail2[Delete ZIP, throw error] Check3 -->|no| Fail3[Delete ZIP, throw error] Check1 -->|yes| Extract1[Extract whitelisted .so files] Check2 -->|yes| Extract2[Extract whitelisted .so files] Check3 -->|yes| Extract3[Extract whitelisted .so files] Extract1 --> Flatten1[Flatten to jniLibs/arm64-v8a] Extract2 --> Flatten2[Flatten to jniLibs/armeabi-v7a] Extract3 --> Flatten3[Flatten to jniLibs/x86_64] Flatten1 --> Merge[Gradle mergeJniLibFolders] Flatten2 --> Merge Flatten3 --> Merge Skip --> Merge Merge --> Build[Continue build]Last reviewed commit: 1c54218
(2/5) Greptile learns from your feedback when you react with thumbs up/down!