Skip to content

Commit 0a0f5af

Browse files
risunCodeclaude
andcommitted
refactor: simplify over-engineered code and add navigation improvements
- Consolidate validation: merge ShellCommandPolicy into CommandValidator - Simplify ShellService: remove session tokens, reduce 244→85 lines (65%) - Simplify SafetyValidator: reduce packages 110+→30, remove injection checks - Remove rollback feature from action history - Add bottom NavigationBar to Dashboard (Home, Tools, Settings) - Add Material3 loading indicators to all screens - Update CHANGELOG with refactoring details Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 016a0ea commit 0a0f5af

File tree

13 files changed

+542
-624
lines changed

13 files changed

+542
-624
lines changed
Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
# Design Document
2+
3+
## Overview
4+
5+
Refactoring akan menghapus layer abstraksi yang tidak perlu sambil mempertahankan fungsionalitas dan keamanan. Pendekatan utama adalah konsolidasi validasi ke satu layer, simplifikasi Shizuku service dengan mengandalkan built-in security, dan menghapus fitur yang tidak terpakai.
6+
7+
## Architecture Changes
8+
9+
### Before
10+
```
11+
ShellManager → ShellCommandPolicy (validation)
12+
→ ShellService → ShellCommandPolicy (validation again)
13+
→ SafetyValidator (package + injection check)
14+
15+
ShellService: Session management (UUID, ConcurrentHashMap, TTL)
16+
: Manual thread management (AtomicReference, thread spawning)
17+
18+
AppManager → SafetyValidator (package validation)
19+
→ ActionHistoryStore (with rollback metadata)
20+
```
21+
22+
### After
23+
```
24+
ShellManager → CommandValidator (single validation layer)
25+
→ ShellService (simple UID check only)
26+
27+
ShellService: Direct UID validation via Shizuku
28+
: Simple ProcessBuilder.exec()
29+
30+
AppManager → SafetyValidator (package safety only)
31+
→ ActionHistoryStore (simple logging)
32+
```
33+
34+
## Components and Interfaces
35+
36+
### 1. ShellService Simplification
37+
38+
**Remove:**
39+
- `openSession()` / `closeSession()` methods
40+
- Session token parameter from `exec()` and `execReturnCode()`
41+
- ConcurrentHashMap, UUID, TTL cleanup logic
42+
- Manual thread spawning for stdout/stderr
43+
- AtomicReference wrappers
44+
- Complex stream truncation logic
45+
46+
**Keep:**
47+
- Direct UID validation via `Binder.getCallingUid()`
48+
- Timeout handling (15s)
49+
- Error handling
50+
51+
**New AIDL Interface:**
52+
```java
53+
interface IShellService {
54+
String exec(String command);
55+
int execReturnCode(String command);
56+
}
57+
```
58+
59+
**Simplified Execution:**
60+
```kotlin
61+
private fun executeCommand(command: String): String {
62+
val process = ProcessBuilder("sh", "-c", command)
63+
.redirectErrorStream(true)
64+
.start()
65+
66+
val completed = process.waitFor(15, TimeUnit.SECONDS)
67+
if (!completed) {
68+
process.destroyForcibly()
69+
throw TimeoutException("Command timeout")
70+
}
71+
72+
val output = process.inputStream.bufferedReader().readText()
73+
if (process.exitValue() != 0) {
74+
throw Exception(output.ifEmpty { "Exit code: ${process.exitValue()}" })
75+
}
76+
return output.trim()
77+
}
78+
```
79+
80+
### 2. Command Validation Consolidation
81+
82+
**Create New:** `CommandValidator` in `core/` package
83+
84+
**Responsibilities:**
85+
- Validate command syntax (forbidden chars, length, shell patterns)
86+
- Validate command whitelist (pm, am, appops, cmd, dumpsys)
87+
- Single source of truth for command validation
88+
89+
**Remove:**
90+
- `ShellCommandPolicy` class entirely
91+
- Command validation from `ShellService`
92+
- Injection detection from `SafetyValidator`
93+
94+
**Integration Point:**
95+
- Called only in `ShellManager.execute()` before sending to service
96+
97+
### 3. SafetyValidator Simplification
98+
99+
**Keep Only:**
100+
- Package name format validation
101+
- Safety level determination (CRITICAL, FORCE_STOP_ONLY, WARNING, SAFE)
102+
- Allowed actions per safety level
103+
104+
**Remove:**
105+
- Injection character detection (handled by CommandValidator)
106+
- Excessive hardcoded package lists
107+
108+
**Reduced Critical Packages (25 items):**
109+
```kotlin
110+
private val CRITICAL_PACKAGES = setOf(
111+
// Self
112+
"com.appcontrolx",
113+
114+
// Core Android
115+
"android",
116+
"com.android.systemui",
117+
"com.android.settings",
118+
"com.android.phone",
119+
"com.android.shell",
120+
"com.android.bluetooth",
121+
"com.android.wifi",
122+
"com.android.networkstack",
123+
"com.android.permissioncontroller",
124+
"com.android.packageinstaller",
125+
"com.android.webview",
126+
127+
// Google Core
128+
"com.google.android.gms",
129+
"com.google.android.gsf",
130+
"com.android.vending",
131+
132+
// Vendor Launchers (critical only)
133+
"com.android.launcher3",
134+
"com.miui.home",
135+
"com.sec.android.app.launcher",
136+
"com.oppo.launcher",
137+
"com.huawei.android.launcher",
138+
139+
// Root/Shizuku
140+
"com.topjohnwu.magisk",
141+
"rikka.shizuku",
142+
"moe.shizuku.privileged.api",
143+
"me.weishu.kernelsu"
144+
)
145+
```
146+
147+
**Force-Stop Only (5 items):**
148+
```kotlin
149+
private val FORCE_STOP_ONLY_PACKAGES = setOf(
150+
"com.miui.powerkeeper",
151+
"com.samsung.android.lool",
152+
"com.coloros.safecenter",
153+
"com.huawei.systemmanager",
154+
"com.google.android.apps.adm"
155+
)
156+
```
157+
158+
### 4. ShellManager Updates
159+
160+
**Changes:**
161+
- Remove session token management
162+
- Add CommandValidator call before execution
163+
- Simplify Shizuku service binding (no session open/close)
164+
- Update `executeViaShizuku()` to call new AIDL interface
165+
166+
**Flow:**
167+
```kotlin
168+
suspend fun execute(command: String): Result<String> {
169+
// Single validation point
170+
CommandValidator.validate(command).getOrElse {
171+
return Result.failure(it)
172+
}
173+
174+
when (currentMode) {
175+
ExecutionMode.ROOT -> executeViaRoot(command)
176+
ExecutionMode.SHIZUKU -> executeViaShizuku(command)
177+
ExecutionMode.NONE -> Result.failure(...)
178+
}
179+
}
180+
181+
private fun executeViaShizuku(command: String): Result<String> {
182+
val service = getShizukuService() ?: return Result.failure(...)
183+
return try {
184+
val output = service.exec(command) // No token needed
185+
Result.success(output)
186+
} catch (e: Exception) {
187+
Result.failure(e)
188+
}
189+
}
190+
```
191+
192+
### 5. ActionHistory Simplification
193+
194+
**Remove from ActionHistoryItem:**
195+
- `canRollback: Boolean` field
196+
197+
**Remove from AppManager:**
198+
- `isRollbackSupported()` method
199+
- Rollback logic in `executeAction()`
200+
201+
**Simplified History Storage:**
202+
```kotlin
203+
actionHistoryStore.addAction(
204+
ActionHistoryItem(
205+
packageName = packageName,
206+
appName = resolveAppName(packageName),
207+
action = action,
208+
timestamp = System.currentTimeMillis()
209+
)
210+
)
211+
```
212+
213+
## Data Models
214+
215+
### Modified Models
216+
217+
**ActionHistoryItem:**
218+
```kotlin
219+
@Serializable
220+
data class ActionHistoryItem(
221+
val packageName: String,
222+
val appName: String,
223+
val action: AppAction,
224+
val timestamp: Long
225+
)
226+
```
227+
228+
**IShellService.aidl:**
229+
```java
230+
interface IShellService {
231+
String exec(String command);
232+
int execReturnCode(String command);
233+
}
234+
```
235+
236+
## Error Handling
237+
238+
**Maintained:**
239+
- Timeout handling (15s for commands)
240+
- Exit code validation
241+
- Exception propagation
242+
- Security exceptions for unauthorized operations
243+
244+
**Simplified:**
245+
- No session expiry errors
246+
- No token mismatch errors
247+
- Direct error messages from command execution
248+
249+
## Testing Strategy
250+
251+
**Validation:**
252+
- Build project without errors
253+
- Verify lint passes
254+
- Test all app actions (freeze, unfreeze, force-stop, etc.)
255+
- Verify security validation still blocks dangerous operations
256+
257+
**Manual Testing:**
258+
- Test with ROOT mode
259+
- Test with SHIZUKU mode
260+
- Verify action history still logs correctly
261+
- Confirm UI remains unchanged
262+
263+
## Migration Notes
264+
265+
**Breaking Changes:**
266+
- AIDL interface changed (Shizuku service needs rebuild)
267+
- ActionHistoryItem serialization format changed (old history incompatible)
268+
269+
**Non-Breaking:**
270+
- All app functionality remains identical
271+
- UI unchanged
272+
- User experience unchanged
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# Requirements Document
2+
3+
## Introduction
4+
5+
AppControlX memiliki beberapa layer yang over-engineered yang menambah kompleksitas tanpa memberikan value signifikan. Refactoring ini bertujuan untuk menyederhanakan codebase dengan menghapus abstraksi berlebihan, menggabungkan validasi ganda, dan menghilangkan fitur yang tidak terpakai, sambil tetap mempertahankan fungsionalitas inti dan keamanan aplikasi.
6+
7+
## Requirements
8+
9+
### Requirement 1: Simplify Shizuku Session Management
10+
11+
**User Story:** As a developer, I want to remove the complex session token system in ShellService, so that the code is simpler and easier to maintain while still secure.
12+
13+
#### Acceptance Criteria
14+
15+
1. WHEN ShellService receives a command request THEN it SHALL validate caller UID directly without session tokens
16+
2. WHEN a command is executed via Shizuku THEN it SHALL rely on Shizuku's built-in security instead of custom session management
17+
3. WHEN ShellService is refactored THEN it SHALL remove ConcurrentHashMap, UUID tokens, and TTL cleanup logic
18+
4. WHEN the simplified service is used THEN it SHALL maintain the same security guarantees as before
19+
20+
### Requirement 2: Merge Duplicate Command Validation
21+
22+
**User Story:** As a developer, I want to consolidate command validation into a single layer, so that there's no redundant validation logic across multiple classes.
23+
24+
#### Acceptance Criteria
25+
26+
1. WHEN a shell command is prepared for execution THEN it SHALL be validated only once
27+
2. WHEN validation logic is consolidated THEN it SHALL be placed in ShellManager as the single source of truth
28+
3. WHEN ShellCommandPolicy is removed THEN its validation rules SHALL be integrated into the remaining validator
29+
4. WHEN SafetyValidator is simplified THEN it SHALL focus only on package-level safety checks, not command syntax
30+
31+
### Requirement 3: Simplify ShellService Stream Handling
32+
33+
**User Story:** As a developer, I want to remove manual thread management for process streams, so that the code is more maintainable and less error-prone.
34+
35+
#### Acceptance Criteria
36+
37+
1. WHEN ShellService executes a command THEN it SHALL use simple ProcessBuilder.exec() without manual thread spawning
38+
2. WHEN process output is read THEN it SHALL use standard InputStream reading without AtomicReference wrappers
39+
3. WHEN output truncation is needed THEN it SHALL use simple string length checks instead of complex stream limiting
40+
4. WHEN the simplified execution completes THEN it SHALL maintain the same timeout and error handling behavior
41+
42+
### Requirement 4: Reduce SafetyValidator Hardcoded Lists
43+
44+
**User Story:** As a developer, I want to minimize the hardcoded package lists in SafetyValidator, so that the code is more maintainable and focused on truly critical packages.
45+
46+
#### Acceptance Criteria
47+
48+
1. WHEN SafetyValidator is refactored THEN it SHALL keep only 20-30 truly critical system packages
49+
2. WHEN a package is in the critical list THEN it SHALL be essential for system stability (SystemUI, Settings, Phone, Bluetooth, etc.)
50+
3. WHEN vendor-specific packages are evaluated THEN only the most critical ones SHALL be retained
51+
4. WHEN injection detection is reviewed THEN redundant checks already covered by command validation SHALL be removed
52+
53+
### Requirement 5: Remove Unused Rollback Feature
54+
55+
**User Story:** As a developer, I want to remove the rollback feature from ActionHistoryStore, so that the codebase doesn't carry unused complexity.
56+
57+
#### Acceptance Criteria
58+
59+
1. WHEN ActionHistoryItem is simplified THEN it SHALL remove the `canRollback` field
60+
2. WHEN action history is stored THEN it SHALL only log the action without rollback metadata
61+
3. WHEN AppManager executes actions THEN it SHALL not check or set rollback support
62+
4. WHEN the refactoring is complete THEN action history SHALL still be viewable but without rollback UI/logic
63+
64+
### Requirement 6: Maintain Existing Functionality
65+
66+
**User Story:** As a user, I want the app to work exactly the same after refactoring, so that I don't experience any regressions or breaking changes.
67+
68+
#### Acceptance Criteria
69+
70+
1. WHEN the refactoring is complete THEN all app actions (freeze, unfreeze, force-stop, etc.) SHALL work identically
71+
2. WHEN commands are executed THEN security validation SHALL still prevent dangerous operations
72+
3. WHEN the app is built THEN it SHALL compile without errors and pass existing lint checks
73+
4. WHEN the app runs THEN UI and navigation SHALL remain unchanged

0 commit comments

Comments
 (0)