made unpack files more aggressive#1300
Conversation
📝 WalkthroughWalkthroughA private utility function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/utils/ContainerUtils.kt (1)
1266-1300: Optional: lowercase defensively and consider a comment on the dual-check intent.The function silently assumes its input is already lowercased (both current callers —
filterExesForUnpackingat line 1231 andgetExecutablePriorityat line 1239 — do so). A future caller that forgets would yield silent false negatives (e.g.,Setup.exewould not match"setup"). A one-line.lowercase()makes the contract self-enforcing. Also, the prefix-list and token-set deliberately overlap (the prefix path catches concatenated-without-separator names likevcredistsetupthat the tokenizer treats as a single token, while the token path catches infix matches likegame_setup); a brief comment would prevent well-meaning future deduplication.♻️ Proposed refactor
private fun isSystemExecutable(fileName: String): Boolean { - val baseName = fileName.removeSuffix(".exe") + val baseName = fileName.lowercase().removeSuffix(".exe") + // Two-pass detection: + // 1) prefix check catches concatenated names the tokenizer can't split (e.g. "vcredistsetup") + // 2) token check catches denylist words appearing as a separated token (e.g. "game_setup") val strongPrefixes = listOf(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt` around lines 1266 - 1300, isSystemExecutable currently assumes fileName is already lowercase which can cause false negatives; update the function (isSystemExecutable) to defensively lowercase the input (e.g., operate on val baseName = fileName.lowercase().removeSuffix(".exe")) and add a short comment above the strongPrefixes/denylist logic explaining the dual-check intent (prefixes catch concatenated names like "vcredistsetup" while tokenization catches infix/underscore-separated names like "game_setup") so future maintainers don’t remove one of the checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`:
- Around line 1266-1300: isSystemExecutable currently assumes fileName is
already lowercase which can cause false negatives; update the function
(isSystemExecutable) to defensively lowercase the input (e.g., operate on val
baseName = fileName.lowercase().removeSuffix(".exe")) and add a short comment
above the strongPrefixes/denylist logic explaining the dual-check intent
(prefixes catch concatenated names like "vcredistsetup" while tokenization
catches infix/underscore-separated names like "game_setup") so future
maintainers don’t remove one of the checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f54006d8-988f-4a0e-ba57-8abc5c7d13a1
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/utils/ContainerUtils.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/utils/ContainerUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerUtils.kt:1290">
P2: `install` is missing from the token denylist, so installer executables like `game-install.exe` can slip through as runnable targets.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "unins", | ||
| "uninstall", | ||
| "setup", | ||
| "installer", |
There was a problem hiding this comment.
P2: install is missing from the token denylist, so installer executables like game-install.exe can slip through as runnable targets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/utils/ContainerUtils.kt, line 1290:
<comment>`install` is missing from the token denylist, so installer executables like `game-install.exe` can slip through as runnable targets.</comment>
<file context>
@@ -1264,12 +1264,38 @@ object ContainerUtils {
+ "unins",
+ "uninstall",
+ "setup",
+ "installer",
+ "redist",
+ "vcredist",
</file context>
| "installer", | |
| "install", | |
| "installer", |
1 participant
Description
Unpack more files to reduce application load errors
Recording
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Tightened
isSystemExecutabledetection so we unpack more files and avoid app load errors. Replaced broad substring checks with targeted prefix and token matching for known installer/redistributable/crash-report binaries.unins,uninstall,setup,install,redist,vcredist,vc_redist,dxsetup,directx,crashhandler,crashreporter.viewer,tool,compiler, and store clients).Written for commit a780a5a. Summary will update on new commits.
Summary by CodeRabbit