-
Notifications
You must be signed in to change notification settings - Fork 10
feat: add pincode protection to settings #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add pincode protection to settings #38
Conversation
📝 WalkthroughWalkthroughA Makefile is added with Android build management targets, and PIN-code-based protection for settings access is implemented via shared preferences, password verification dialogs, and new preference UI elements across MainActivity, SettingsFragment, and the settings XML configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainActivity
participant Dialog as PIN Dialog
participant SharedPref as SharedPreferences
participant SettingsActivity
rect rgb(200, 220, 255)
note over User,SettingsActivity: PIN Protection - First Access (Settings Enabled)
User->>MainActivity: onSettingsCommand()
MainActivity->>SharedPref: Check for saved PIN
SharedPref-->>MainActivity: PIN exists
MainActivity->>Dialog: promptForPasswordVerification()
Dialog->>User: Show numeric PIN input
User->>Dialog: Enter PIN
Dialog->>Dialog: Verify PIN
alt Correct PIN
Dialog->>SettingsActivity: openSettings()
SettingsActivity-->>User: Settings Activity launched
else Incorrect PIN
Dialog->>User: Show toast error
end
end
rect rgb(200, 255, 220)
note over User,SettingsActivity: PIN Setup - In Settings Fragment
User->>SettingsActivity: Enable settingsPincode switch
SettingsActivity->>Dialog: promptForPasswordCreation()
Dialog->>User: Show confirmation dialog
User->>Dialog: Confirm PIN creation
Dialog->>User: Show numeric PIN input
User->>Dialog: Enter PIN
Dialog->>Dialog: Validate non-empty
Dialog->>SharedPref: Save PIN
Dialog->>SettingsActivity: Update switch state
Dialog->>User: Show success toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @app/src/main/java/com/immichframe/immichframe/MainActivity.kt:
- Around line 729-750: promptForPasswordVerification currently may not reliably
mask input, forces non-cancelable dialogs, and lacks brute-force protection;
update it to use a masked input reliably (ensure EditText uses
passwordTransformationMethod fallback when
InputType.TYPE_NUMBER_VARIATION_PASSWORD doesn't mask), change
setCancelable(false) to setCancelable(true) so users can dismiss the dialog, and
add simple rate-limiting state (e.g. private vars failedPinAttempts and
pinLockoutUntil) checked at the start of promptForPasswordVerification to refuse
attempts while locked and to increment failedPinAttempts on wrong PIN, reset
failedPinAttempts to 0 on success (openSettings), and set pinLockoutUntil to
now+lockoutDuration after N failures to enforce a temporary lockout.
In @app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt:
- Around line 180-209: The PIN creation flow in promptForPasswordCreation lacks
confirmation, minimal length validation, and reliable masking; update
promptForPasswordCreation to enforce a minimum length (e.g., >=4), show a "Next"
action that calls a new promptForPasswordConfirmation(originalPassword,
chkSettingsPincode) which asks the user to re-enter the PIN and only saves to
PreferenceManager.getDefaultSharedPreferences when both entries match; ensure
both EditText instances enable consistent masking (e.g.,
setTransformationMethod(PasswordTransformationMethod.getInstance()) and/or
proper inputType) and add user-facing hints/toasts for "PIN too short" and "PINs
do not match" while unchecking chkSettingsPincode on failure.
🧹 Nitpick comments (2)
Makefile (1)
1-58: Consider addressing static analysis hints for best practices.The Makefile is functional, but checkmake flags two minor style issues:
- The
helptarget body exceeds the recommended length (10 lines vs. 5).- The conventional
alltarget is missing.These are optional improvements that enhance maintainability and align with Make conventions.
♻️ Optional improvements
Add an
alltarget and consider shortening the help output:-.PHONY: build clean install install-debug install-release test help +.PHONY: all build clean install install-debug install-release test help + +# Default: build debug +all: build # Default target help: @echo "Available targets:" - @echo " build - Build debug APK" - @echo " build-release - Build release APK" - @echo " clean - Clean build artifacts" - @echo " install - Install debug APK to connected device" - @echo " install-release- Install release APK to connected device" - @echo " uninstall - Uninstall app from connected device" - @echo " test - Run tests" - @echo " lint - Run lint checks" - @echo " assemble - Build all variants" + @echo " all, build, build-release, clean, install, install-release," + @echo " uninstall, test, lint, assemble, run"app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
718-721: Document the intentional RPC bypass in code comments.The PR description mentions that RPC access bypasses PIN protection, but this isn't evident from the code. Adding a comment here improves maintainability.
📝 Suggested documentation
private fun settingsActionFromRpc() { - // RPC access bypasses password protection + // RPC access bypasses PIN protection by design. + // Use case: Remote administration when device is locked or inaccessible. + // Note: Secure your RPC endpoint at the network level. openSettings() }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefileapp/src/main/java/com/immichframe/immichframe/MainActivity.ktapp/src/main/java/com/immichframe/immichframe/SettingsFragment.ktapp/src/main/res/xml/settings_view.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
stopImageTimer(374-377)
🪛 checkmake (0.2.2)
Makefile
[warning] 4-4: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 1-1: Missing required phony target "all"
(minphony)
🔇 Additional comments (3)
app/src/main/res/xml/settings_view.xml (1)
32-43: LGTM! Clear separation of tamper protection options.The new "Tamper Protection" category cleanly groups PIN protection and full settings lock. The updated title for
settingsLock("Fully disable/lock access...") effectively distinguishes it from the PIN option.app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
705-716: LGTM! Clean separation of authenticated vs. unauthenticated access.The logic correctly checks for a saved PIN and routes to verification or direct access accordingly.
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt (1)
82-111: LGTM! Proper confirmation flow for security-sensitive setting.The two-stage confirmation (intent verification, then PIN creation) provides good UX for a security feature. The disable path correctly removes the stored PIN.
| private fun promptForPasswordVerification(correctPassword: String) { | ||
| val input = EditText(this) | ||
| input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD | ||
|
|
||
| AlertDialog.Builder(this) | ||
| .setTitle("Enter Pincode") | ||
| .setMessage("Enter your pincode to access settings:") | ||
| .setView(input) | ||
| .setPositiveButton("OK") { _, _ -> | ||
| val enteredPassword = input.text.toString() | ||
| if (enteredPassword == correctPassword) { | ||
| openSettings() | ||
| } else { | ||
| Toast.makeText(this, "Incorrect pincode", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| .setNegativeButton("Cancel") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .setCancelable(false) | ||
| .show() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider UX and security improvements to PIN verification.
Three concerns:
-
Input masking:
TYPE_NUMBER_VARIATION_PASSWORDmay not mask input on all Android versions/devices. Consider testing or adding a fallback. -
Dialog cancelability: Line 748 sets
setCancelable(false), forcing users to either enter a PIN or click Cancel. This could be frustrating if triggered accidentally. ConsidersetCancelable(true). -
Rate limiting: No protection against brute-force attempts. An attacker could try all 10,000 4-digit PINs rapidly.
🔒 Recommended improvements
For rate limiting, consider adding a simple lockout after failed attempts:
private var failedPinAttempts = 0
private var pinLockoutUntil = 0L
private fun promptForPasswordVerification(correctPassword: String) {
// Check lockout
if (System.currentTimeMillis() < pinLockoutUntil) {
val remainingSec = (pinLockoutUntil - System.currentTimeMillis()) / 1000
Toast.makeText(this, "Too many attempts. Try again in ${remainingSec}s", Toast.LENGTH_SHORT).show()
return
}
val input = EditText(this)
input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD
AlertDialog.Builder(this)
.setTitle("Enter Pincode")
.setMessage("Enter your pincode to access settings:")
.setView(input)
.setPositiveButton("OK") { _, _ ->
val enteredPassword = input.text.toString()
if (enteredPassword == correctPassword) {
failedPinAttempts = 0
openSettings()
} else {
failedPinAttempts++
if (failedPinAttempts >= 3) {
pinLockoutUntil = System.currentTimeMillis() + 30_000 // 30s lockout
Toast.makeText(this, "Too many failed attempts. Locked for 30 seconds.", Toast.LENGTH_LONG).show()
} else {
Toast.makeText(this, "Incorrect pincode", Toast.LENGTH_SHORT).show()
}
}
}
.setNegativeButton("Cancel") { dialog, _ ->
dialog.dismiss()
}
.setCancelable(true) // Allow dismissal
.show()
}🤖 Prompt for AI Agents
In @app/src/main/java/com/immichframe/immichframe/MainActivity.kt around lines
729 - 750, promptForPasswordVerification currently may not reliably mask input,
forces non-cancelable dialogs, and lacks brute-force protection; update it to
use a masked input reliably (ensure EditText uses passwordTransformationMethod
fallback when InputType.TYPE_NUMBER_VARIATION_PASSWORD doesn't mask), change
setCancelable(false) to setCancelable(true) so users can dismiss the dialog, and
add simple rate-limiting state (e.g. private vars failedPinAttempts and
pinLockoutUntil) checked at the start of promptForPasswordVerification to refuse
attempts while locked and to increment failedPinAttempts on wrong PIN, reset
failedPinAttempts to 0 on success (openSettings), and set pinLockoutUntil to
now+lockoutDuration after N failures to enforce a temporary lockout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of these are particular concerns for the use-case of the frame in a residential setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juanjsebgarcia Understood! For a residential frame protecting against children, the current implementation is appropriate. Thank you for clarifying the use case.
| private fun promptForPasswordCreation(chkSettingsPincode: SwitchPreferenceCompat?) { | ||
| val input = android.widget.EditText(requireContext()) | ||
| input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD | ||
|
|
||
| AlertDialog.Builder(requireContext()) | ||
| .setTitle("Create Pincode") | ||
| .setMessage("Enter a pincode to protect settings access:") | ||
| .setView(input) | ||
| .setPositiveButton("Set") { _, _ -> | ||
| val password = input.text.toString() | ||
| if (password.isEmpty()) { | ||
| Toast.makeText(requireContext(), "Pincode cannot be empty", Toast.LENGTH_SHORT).show() | ||
| } else { | ||
| // Save the password | ||
| val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext()) | ||
| sharedPrefs.edit() | ||
| .putString("settings_pincode", password) | ||
| .apply() | ||
|
|
||
| // Update the switch | ||
| chkSettingsPincode?.isChecked = true | ||
|
|
||
| Toast.makeText(requireContext(), "Pincode protection enabled", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| .setNegativeButton("Cancel") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .show() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Enhance PIN validation and user experience.
Several improvements would strengthen this feature:
-
No PIN confirmation: Users should re-enter the PIN to prevent typos (especially since input is masked).
-
Minimal validation: Line 190 only checks
isEmpty(). Consider enforcing a minimum length (e.g., 4 digits) for basic security. -
Input masking:
TYPE_NUMBER_VARIATION_PASSWORDmay not mask input consistently across devices (same concern as in MainActivity).
🔐 Recommended improvements
Add PIN confirmation and minimum length validation:
private fun promptForPasswordCreation(chkSettingsPincode: SwitchPreferenceCompat?) {
val input = android.widget.EditText(requireContext())
input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD
input.hint = "Minimum 4 digits"
AlertDialog.Builder(requireContext())
.setTitle("Create Pincode")
.setMessage("Enter a pincode to protect settings access:")
.setView(input)
.setPositiveButton("Next") { _, _ ->
val password = input.text.toString()
if (password.length < 4) {
Toast.makeText(requireContext(), "Pincode must be at least 4 digits", Toast.LENGTH_SHORT).show()
} else {
// Prompt for confirmation
promptForPasswordConfirmation(password, chkSettingsPincode)
}
}
.setNegativeButton("Cancel") { dialog, _ ->
dialog.dismiss()
}
.show()
}
private fun promptForPasswordConfirmation(originalPassword: String, chkSettingsPincode: SwitchPreferenceCompat?) {
val confirmInput = android.widget.EditText(requireContext())
confirmInput.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD
AlertDialog.Builder(requireContext())
.setTitle("Confirm Pincode")
.setMessage("Re-enter your pincode:")
.setView(confirmInput)
.setPositiveButton("Set") { _, _ ->
val confirmedPassword = confirmInput.text.toString()
if (confirmedPassword == originalPassword) {
// Save the password
val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext())
sharedPrefs.edit()
.putString("settings_pincode", originalPassword)
.apply()
// Update the switch
chkSettingsPincode?.isChecked = true
Toast.makeText(requireContext(), "Pincode protection enabled", Toast.LENGTH_SHORT).show()
} else {
Toast.makeText(requireContext(), "Pincodes do not match", Toast.LENGTH_SHORT).show()
chkSettingsPincode?.isChecked = false
}
}
.setNegativeButton("Cancel") { dialog, _ ->
dialog.dismiss()
}
.show()
}🤖 Prompt for AI Agents
In @app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt around
lines 180 - 209, The PIN creation flow in promptForPasswordCreation lacks
confirmation, minimal length validation, and reliable masking; update
promptForPasswordCreation to enforce a minimum length (e.g., >=4), show a "Next"
action that calls a new promptForPasswordConfirmation(originalPassword,
chkSettingsPincode) which asks the user to re-enter the PIN and only saves to
PreferenceManager.getDefaultSharedPreferences when both entries match; ensure
both EditText instances enable consistent masking (e.g.,
setTransformationMethod(PasswordTransformationMethod.getInstance()) and/or
proper inputType) and add user-facing hints/toasts for "PIN too short" and "PINs
do not match" while unchecking chkSettingsPincode on failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation is not designed to be high security, just to keep the innocent out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juanjsebgarcia, that makes perfect sense! For the use case of keeping children out, the current implementation is appropriate. Thank you for clarifying the design intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @app/src/main/java/com/immichframe/immichframe/Helpers.kt:
- Around line 419-429: applyGammaToBitmap currently documents that it always
recycles the input bitmap but on the size-check path (width * height >
maxPixels) it returns the original bitmap without recycling; fix by returning a
new copy of the bitmap and then recycling the original to preserve the contract:
create a mutable copy of the input (use the bitmap.copy(...) or equivalent to
match Android Bitmap API), call bitmap.recycle() on the original, and return the
copy; update the size-check branch inside applyGammaToBitmap (referencing width,
height, maxPixels) to perform these steps and handle copy failures safely (e.g.,
if copy is null, fall back to returning the original without recycling).
In @app/src/main/java/com/immichframe/immichframe/PowerButtonReturnService.kt:
- Around line 67-74: The code calls
packageManager.getLaunchIntentForPackage(packageName) into returnIntent and then
calls startActivity(returnIntent) without handling a null; update
PowerButtonReturnService to check returnIntent for null after retrieving it
(before calling addFlags/startActivity), and if null log a warning including
packageName and skip startActivity (still call stopSelf or handle cleanup as
appropriate) to avoid the NPE from startActivity when getLaunchIntentForPackage
returns null.
In @app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt:
- Around line 208-250: The promptForPasswordCreation function is missing its
terminating brace, causing a syntax/compilation error; fix it by adding the
missing closing brace '}' immediately after the AlertDialog.show() call to
properly end promptForPasswordCreation (ensure the brace is placed before the
@Suppress("DEPRECATION") annotation), and verify indentation so the subsequent
onDisplayPreferenceDialog override remains outside the function.
In @app/src/main/java/com/immichframe/immichframe/SettingsTimeoutReceiver.kt:
- Around line 24-27: The receiver's code calls
context.packageManager.getLaunchIntentForPackage(...) without null-check and may
crash; update SettingsTimeoutReceiver to handle a null returnIntent (as done in
PowerButtonReturnService) by checking returnIntent for null before calling
addFlags/startActivity and log or silently return if null, ensuring the method
exits gracefully instead of dereferencing a null Intent.
🧹 Nitpick comments (7)
app/src/main/res/layout/preference_seekbar.xml (1)
13-13: Externalize hardcoded strings for internationalization.The hardcoded text values ("0", "-100", "100", "Reset to Default") should be moved to
strings.xmlto support localization and follow Android best practices.Example string resource definitions
Add to
res/values/strings.xml:<string name="seekbar_default_value">0</string> <string name="seekbar_min_value">-100</string> <string name="seekbar_max_value">100</string> <string name="seekbar_reset_button">Reset to Default</string>Then reference them in the layout:
- android:text="0" + android:text="@string/seekbar_default_value"- android:text="-100" + android:text="@string/seekbar_min_value"- android:text="100" + android:text="@string/seekbar_max_value"- android:text="Reset to Default" + android:text="@string/seekbar_reset_button"Also applies to: 34-34, 43-43, 55-55
app/src/main/res/xml/settings_view.xml (2)
36-39: Note: Pincode stored in plaintext per PR objectives.The PR objectives explicitly state that the pincode is stored in plaintext in a non-secure location. This is a known limitation acknowledged by the author. For production use, consider migrating to Android's EncryptedSharedPreferences or the Keystore system for more secure credential storage.
If you'd like to explore more secure storage options in the future, I can provide implementation guidance for EncryptedSharedPreferences.
51-87: Consider adding dependency attributes for better UX.The individual
SeekBarPreferenceitems (brightness, contrast, RGB channels, gamma) could includeandroid:dependency="imageAdjustments"to automatically disable them when the parent "Image Adjustments?" switch is turned off. This provides clearer visual feedback to users about which settings are active.Example dependency attribute
<com.immichframe.immichframe.SeekBarPreference android:key="image_brightness" android:title="Brightness" android:defaultValue="0" + android:dependency="imageAdjustments" app:min="-100" app:max="100" />Apply similarly to the other SeekBarPreference elements.
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
531-545: Consider extracting duplicated WebView filter application logic.The WebView color filter application code (with hardware/software layer fallback) is duplicated between the initialization block (lines 531-545) and the
reapplyImageFilters()method (lines 577-589). Extracting this into a private helper method would improve maintainability and reduce the risk of inconsistencies.♻️ Proposed refactor
private fun applyFilterToWebView() { if (!::webView.isInitialized) return val filter = Helpers.getImageAdjustmentFilter(this) if (filter != null) { val paint = Paint() paint.colorFilter = filter try { webView.setLayerType(View.LAYER_TYPE_HARDWARE, paint) } catch (e: Exception) { Log.w("ScreenSaverService", "Hardware layer failed, falling back to software: ${e.message}") webView.setLayerType(View.LAYER_TYPE_SOFTWARE, paint) } } else { webView.setLayerType(View.LAYER_TYPE_HARDWARE, null) } }Then replace both occurrences:
- // Apply color filter to WebView using layer rendering - val filter = Helpers.getImageAdjustmentFilter(this) - if (filter != null) { - val paint = Paint() - paint.colorFilter = filter - try { - webView.setLayerType(View.LAYER_TYPE_HARDWARE, paint) - } catch (e: Exception) { - // Fallback to software layer if hardware fails - Log.w("ScreenSaverService", "Hardware layer failed, falling back to software: ${e.message}") - webView.setLayerType(View.LAYER_TYPE_SOFTWARE, paint) - } - } else { - webView.setLayerType(View.LAYER_TYPE_HARDWARE, null) - } + applyFilterToWebView()And in
reapplyImageFilters():- } else if (::webView.isInitialized) { - // Reapply filter to WebView (only if initialized) - val filter = Helpers.getImageAdjustmentFilter(this) - if (filter != null) { - val paint = Paint() - paint.colorFilter = filter - try { - webView.setLayerType(View.LAYER_TYPE_HARDWARE, paint) - } catch (e: Exception) { - Log.w("ScreenSaverService", "Hardware layer failed, falling back to software: ${e.message}") - webView.setLayerType(View.LAYER_TYPE_SOFTWARE, paint) - } - } else { - webView.setLayerType(View.LAYER_TYPE_HARDWARE, null) - } + } else { + applyFilterToWebView() }Also applies to: 577-589
app/src/main/java/com/immichframe/immichframe/PowerButtonReturnService.kt (1)
31-33: Add receiver export flag for Android 13+ compatibility.Starting with Android 13 (API 33),
registerReceiverrequires specifying whether the receiver should be exported. Since this is an internal receiver for system broadcasts, useRECEIVER_NOT_EXPORTED.Proposed fix
// Register broadcast receiver for screen off events val filter = IntentFilter(Intent.ACTION_SCREEN_OFF) - registerReceiver(screenOffReceiver, filter) + if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.TIRAMISU) { + registerReceiver(screenOffReceiver, filter, Context.RECEIVER_NOT_EXPORTED) + } else { + registerReceiver(screenOffReceiver, filter) + }app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
597-612: Consider extracting duplicated WebView filter logic.The WebView filter application logic (lines 597-612) is duplicated in
reapplyImageFilters()(lines 642-656). Consider extracting to a private helper method to reduce duplication and maintenance burden.Proposed refactor
private fun applyFilterToWebView() { val filter = Helpers.getImageAdjustmentFilter(this) if (filter != null) { val paint = Paint() paint.colorFilter = filter try { webView.setLayerType(View.LAYER_TYPE_HARDWARE, paint) } catch (e: Exception) { Log.w("MainActivity", "Hardware layer failed, falling back to software: ${e.message}") webView.setLayerType(View.LAYER_TYPE_SOFTWARE, paint) } } else { webView.setLayerType(View.LAYER_TYPE_HARDWARE, null) } }Then call
applyFilterToWebView()from both locations.Also applies to: 636-658
app/src/main/java/com/immichframe/immichframe/SeekBarPreference.kt (1)
49-55: Use explicit locale for String.format to ensure consistent formatting.Static analysis correctly identifies implicit locale usage. For display values, use
Locale.USorLocale.getDefault()explicitly to ensure consistent decimal formatting across different device locales.Proposed fix
+import java.util.Locale + // In updateSummary(): private fun updateSummary() { summary = if (key == "image_gamma") { - String.format("%.2f", currentValue / 100.0) + String.format(Locale.US, "%.2f", currentValue / 100.0) } else { currentValue.toString() } } // In updateValueText(): private fun updateValueText(value: Int, isGamma: Boolean) { valueText?.text = if (isGamma) { - String.format("%.2f", value / 100.0) + String.format(Locale.US, "%.2f", value / 100.0) } else { value.toString() } }Apply similar changes to lines 81 and 86 for min/max labels.
Also applies to: 120-126
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/immichframe/immichframe/Helpers.ktapp/src/main/java/com/immichframe/immichframe/MainActivity.ktapp/src/main/java/com/immichframe/immichframe/PowerButtonReturnService.ktapp/src/main/java/com/immichframe/immichframe/ScreenSaverService.ktapp/src/main/java/com/immichframe/immichframe/SeekBarPreference.ktapp/src/main/java/com/immichframe/immichframe/SettingsFragment.ktapp/src/main/java/com/immichframe/immichframe/SettingsTimeoutReceiver.ktapp/src/main/java/com/immichframe/immichframe/WidgetProvider.ktapp/src/main/res/layout/preference_seekbar.xmlapp/src/main/res/values/attrs.xmlapp/src/main/res/xml/settings_view.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
reapplyImageFilters(636-658)
🪛 detekt (1.23.8)
app/src/main/java/com/immichframe/immichframe/SeekBarPreference.kt
[warning] 104-104: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 105-105: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 51-51: String.format("%.2f", currentValue / 100.0) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 81-81: String.format("%.1f", preference.min / 100.0) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 86-86: String.format("%.1f", preference.max / 100.0) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 122-122: String.format("%.2f", value / 100.0) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
🔇 Additional comments (14)
app/src/main/AndroidManifest.xml (2)
86-88: LGTM!The
SettingsTimeoutReceiveris properly declared withandroid:exported="false", which is appropriate for a broadcast receiver that handles internal scheduled timeouts. The lack of an intent-filter is correct, as this receiver likely receives explicit intents from the alarm manager.
9-9: No action required. TheSCHEDULE_EXACT_ALARMpermission is already properly handled. The code checkscanScheduleExactAlarms()at line 58 of SettingsTimeoutReceiver.kt and gracefully falls back to inexact alarms (setAndAllowWhileIdle()) when the permission is unavailable, ensuring the timeout feature functions correctly across all Android versions.app/src/main/java/com/immichframe/immichframe/WidgetProvider.kt (1)
93-93: LGTM! Image adjustments correctly integrated into widget update flow.The call to
applyImageAdjustmentsToBitmapis appropriately placed after bitmap quality reduction and before display, ensuring the widget respects user-configured image adjustments.app/src/main/res/values/attrs.xml (1)
1-7: LGTM! Clean attribute declaration.The
SeekBarPreferencestyleable declaration is standard and correctly formatted for use with custom preference attributes.app/src/main/res/xml/settings_view.xml (1)
2-2: LGTM! Namespace declaration required for custom attributes.The
xmlns:appnamespace is necessary for theapp:minandapp:maxattributes used by theSeekBarPreferenceelements.app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (3)
9-9: LGTM! Import required for layer-based color filtering.The
Paintimport supports the new color filter application to WebView and ImageView components.
229-229: LGTM! Filter applied during image transition.Applying the color filter to
imageViewNewensures that the incoming image respects the user's adjustment settings during the crossfade transition.
555-556: LGTM! Filters reapplied after settings load.Calling
reapplyImageFilters()after successfully loading server settings ensures that any already-displayed content receives the current image adjustment filters.app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
774-795: LGTM - PIN verification flow is functional.The PIN verification dialog implementation works correctly. The PIN is compared in plain text as documented in the PR description. The RPC bypass is intentional per the PR objectives.
app/src/main/java/com/immichframe/immichframe/SettingsTimeoutReceiver.kt (1)
55-91: Well-implemented alarm scheduling with proper API-level handling.The code correctly handles Android 12+ exact alarm permission checks with fallback to inexact alarms, and uses appropriate alarm types for older API levels.
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt (1)
239-249: Appropriate handling of deprecated API with suppression.The use of deprecated
setTargetFragmentis correctly suppressed with annotation and documented in comments. This is the standard approach forPreferenceDialogFragmentCompatcompatibility.app/src/main/java/com/immichframe/immichframe/SeekBarPreference.kt (1)
91-118: LGTM - SeekBar listener implementation is correct.The empty
onStartTrackingTouchandonStopTrackingTouchmethods are required by theOnSeekBarChangeListenerinterface. OnlyonProgressChangedneeds implementation here. The static analysis warning about empty blocks can be safely ignored for this pattern.app/src/main/java/com/immichframe/immichframe/Helpers.kt (2)
269-286: Thread-safe caching implementation looks good.The filter cache uses proper synchronization and includes all relevant parameters in the cache key. The early return on cache hit avoids unnecessary filter recreation.
382-409: Clean bitmap lifecycle management with proper recycling.The function correctly manages bitmap lifecycle: creates new result bitmap, applies filter, recycles input, then applies gamma (which handles its own recycling). The documented contract is maintained.
| private fun applyGammaToBitmap(bitmap: Bitmap, gamma: Float): Bitmap { | ||
| val width = bitmap.width | ||
| val height = bitmap.height | ||
|
|
||
| // Safety check: prevent excessive memory allocation | ||
| // Max 4000x4000 = 16M pixels = 64MB for ARGB_8888 | ||
| val maxPixels = 4000 * 4000 | ||
| if (width * height > maxPixels) { | ||
| Log.w("Helpers", "Bitmap too large for gamma correction: ${width}x${height}, skipping gamma") | ||
| return bitmap | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent bitmap recycling contract when size check fails.
The function documents that it always recycles the input bitmap, but when the size check fails (line 426-429), it returns the original bitmap without recycling. This breaks the documented contract and could lead to confusion or memory issues.
Proposed fix - return a copy to maintain contract
// Safety check: prevent excessive memory allocation
// Max 4000x4000 = 16M pixels = 64MB for ARGB_8888
val maxPixels = 4000 * 4000
if (width * height > maxPixels) {
Log.w("Helpers", "Bitmap too large for gamma correction: ${width}x${height}, skipping gamma")
- return bitmap
+ // Return copy and recycle original to maintain contract
+ val copy = bitmap.copy(bitmap.config ?: Bitmap.Config.ARGB_8888, false)
+ bitmap.recycle()
+ return copy
}📝 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.
| private fun applyGammaToBitmap(bitmap: Bitmap, gamma: Float): Bitmap { | |
| val width = bitmap.width | |
| val height = bitmap.height | |
| // Safety check: prevent excessive memory allocation | |
| // Max 4000x4000 = 16M pixels = 64MB for ARGB_8888 | |
| val maxPixels = 4000 * 4000 | |
| if (width * height > maxPixels) { | |
| Log.w("Helpers", "Bitmap too large for gamma correction: ${width}x${height}, skipping gamma") | |
| return bitmap | |
| } | |
| private fun applyGammaToBitmap(bitmap: Bitmap, gamma: Float): Bitmap { | |
| val width = bitmap.width | |
| val height = bitmap.height | |
| // Safety check: prevent excessive memory allocation | |
| // Max 4000x4000 = 16M pixels = 64MB for ARGB_8888 | |
| val maxPixels = 4000 * 4000 | |
| if (width * height > maxPixels) { | |
| Log.w("Helpers", "Bitmap too large for gamma correction: ${width}x${height}, skipping gamma") | |
| // Return copy and recycle original to maintain contract | |
| val copy = bitmap.copy(bitmap.config ?: Bitmap.Config.ARGB_8888, false) | |
| bitmap.recycle() | |
| return copy | |
| } |
🤖 Prompt for AI Agents
In @app/src/main/java/com/immichframe/immichframe/Helpers.kt around lines 419 -
429, applyGammaToBitmap currently documents that it always recycles the input
bitmap but on the size-check path (width * height > maxPixels) it returns the
original bitmap without recycling; fix by returning a new copy of the bitmap and
then recycling the original to preserve the contract: create a mutable copy of
the input (use the bitmap.copy(...) or equivalent to match Android Bitmap API),
call bitmap.recycle() on the original, and return the copy; update the
size-check branch inside applyGammaToBitmap (referencing width, height,
maxPixels) to perform these steps and handle copy failures safely (e.g., if copy
is null, fall back to returning the original without recycling).
| // Launch ImmichFrame and bring it to foreground | ||
| Log.d(TAG, "App in background, bringing to foreground via power button") | ||
| val returnIntent = packageManager.getLaunchIntentForPackage(packageName) | ||
| returnIntent?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) | ||
| startActivity(returnIntent) | ||
|
|
||
| // Stop this service since we're done | ||
| stopSelf() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential NPE if launch intent is null.
getLaunchIntentForPackage can return null. While addFlags uses safe call, startActivity(returnIntent) will crash if returnIntent is null.
Proposed fix
// Launch ImmichFrame and bring it to foreground
Log.d(TAG, "App in background, bringing to foreground via power button")
val returnIntent = packageManager.getLaunchIntentForPackage(packageName)
- returnIntent?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP)
- startActivity(returnIntent)
+ returnIntent?.let {
+ it.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP)
+ startActivity(it)
+ } ?: Log.w(TAG, "Could not get launch intent for package")
// Stop this service since we're done
stopSelf()🤖 Prompt for AI Agents
In @app/src/main/java/com/immichframe/immichframe/PowerButtonReturnService.kt
around lines 67 - 74, The code calls
packageManager.getLaunchIntentForPackage(packageName) into returnIntent and then
calls startActivity(returnIntent) without handling a null; update
PowerButtonReturnService to check returnIntent for null after retrieving it
(before calling addFlags/startActivity), and if null log a warning including
packageName and skip startActivity (still call stopSelf or handle cleanup as
appropriate) to avoid the NPE from startActivity when getLaunchIntentForPackage
returns null.
| // Launch ImmichFrame and bring it to foreground | ||
| val returnIntent = context.packageManager.getLaunchIntentForPackage(context.packageName) | ||
| returnIntent?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) | ||
| context.startActivity(returnIntent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle null launch intent to prevent potential crash.
Similar to PowerButtonReturnService, getLaunchIntentForPackage can return null. The receiver should handle this gracefully.
Proposed fix
// Launch ImmichFrame and bring it to foreground
val returnIntent = context.packageManager.getLaunchIntentForPackage(context.packageName)
- returnIntent?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP)
- context.startActivity(returnIntent)
+ returnIntent?.let {
+ it.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP)
+ context.startActivity(it)
+ } ?: Log.w(TAG, "Could not get launch intent for package")📝 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.
| // Launch ImmichFrame and bring it to foreground | |
| val returnIntent = context.packageManager.getLaunchIntentForPackage(context.packageName) | |
| returnIntent?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) | |
| context.startActivity(returnIntent) | |
| // Launch ImmichFrame and bring it to foreground | |
| val returnIntent = context.packageManager.getLaunchIntentForPackage(context.packageName) | |
| returnIntent?.let { | |
| it.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) | |
| context.startActivity(it) | |
| } ?: Log.w(TAG, "Could not get launch intent for package") |
🤖 Prompt for AI Agents
In @app/src/main/java/com/immichframe/immichframe/SettingsTimeoutReceiver.kt
around lines 24 - 27, The receiver's code calls
context.packageManager.getLaunchIntentForPackage(...) without null-check and may
crash; update SettingsTimeoutReceiver to handle a null returnIntent (as done in
PowerButtonReturnService) by checking returnIntent for null before calling
addFlags/startActivity and log or silently return if null, ensuring the method
exits gracefully instead of dereferencing a null Intent.
Signed-off-by: juanjsebgarcia <github@juangarcia.co.uk>
46e3cfc to
e654ebb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @app/src/main/java/com/immichframe/immichframe/MainActivity.kt:
- Around line 729-750: The PIN dialog in promptForPasswordVerification lacks
rate limiting and a max-attempt lockout and also uses a redundant
setCancelable(false); fix by adding an attempt counter and last-failure
timestamp (e.g., in SharedPreferences) keyed to the settings access, check these
values before verifying and refuse/show a locked message when the temporary
lockout is active, increment attempts on each wrong entry, compute an
exponential backoff delay (or fixed lock duration after N failures), persist the
backoff/lockout state and reset attempts on successful auth, disable the OK
button (or ignore input) while the backoff timer is active, log each failed
attempt with timestamps (using Log or existing analytics/logging helpers), and
remove the redundant setCancelable(false) call from the AlertDialog chain in
promptForPasswordVerification.
In @Makefile:
- Line 1: The .PHONY declaration is missing several targets so Make may treat
them as files; update the .PHONY line (the existing .PHONY: build clean install
install-debug install-release test help) to include the additional phony targets
build-release, assemble, uninstall, lint, and run so Make always executes those
rules instead of treating same‑named files as up‑to‑date.
🧹 Nitpick comments (1)
Makefile (1)
4-4: Consider adding an "all" target per Makefile convention.While not critical, Makefile convention typically includes an
alltarget as the default build target. You could add it after line 3:📝 Optional enhancement
# Default target +all: build + help: @echo "Available targets:"This would provide a standard "make all" command that builds the debug APK, though keeping
helpas the default is also reasonable for developer ergonomics.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefileapp/src/main/java/com/immichframe/immichframe/MainActivity.ktapp/src/main/java/com/immichframe/immichframe/SettingsFragment.ktapp/src/main/res/xml/settings_view.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
app/src/main/java/com/immichframe/immichframe/ScreenSaverService.kt (1)
stopImageTimer(374-377)
🪛 checkmake (0.2.2)
Makefile
[warning] 4-4: Target body for "help" exceeds allowed length of 5 (10).
(maxbodylength)
[warning] 1-1: Missing required phony target "all"
(minphony)
🔇 Additional comments (11)
Makefile (3)
4-15: LGTM! Help target is well-structured.The help target provides clear documentation of available commands. The static analysis warning about body length exceeding 5 lines is a false positive—help targets inherently require multiple lines to document all available targets.
17-54: LGTM! Build and install targets are properly structured.The targets correctly wrap Gradle commands with appropriate dependencies. The explicit build prerequisites for
installandinstall-releasetargets ensure proper ordering and make dependencies clear to users.
57-58: LGTM! Run target correctly launches the app.The
runtarget properly depends oninstalland uses the correctadbcommand to launch MainActivity.app/src/main/res/xml/settings_view.xml (1)
32-43: LGTM! Tamper Protection category added correctly.The XML structure properly closes the Display Options category and introduces the new Tamper Protection preferences with clear titles and appropriate keys that align with the Kotlin implementation.
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (5)
28-28: LGTM! Necessary imports for PIN protection dialog.The new imports support the numeric password input dialog implementation.
Also applies to: 30-30, 34-34
167-167: RPC bypass of PIN protection is intentional.The RPC server correctly calls
settingsActionFromRpc()which bypasses PIN verification, providing a recovery mechanism if the PIN is forgotten.
705-716: Verify plaintext PIN storage is acceptable for your use case.The PIN is stored in plaintext in SharedPreferences, which is accessible to users with device access or root privileges. For the stated use case (preventing children from accessing settings), this may be acceptable, but be aware that it provides minimal security against determined attackers.
718-721: LGTM! RPC bypass method is clear and well-commented.The implementation correctly provides passwordless access via RPC commands, serving as a recovery mechanism.
723-727: LGTM! Good refactoring to extract settings launch logic.Separating the settings launch into its own method improves code clarity and reusability.
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt (2)
75-80: LGTM! Switch initialization correctly syncs with stored state.The implementation properly reflects whether a PIN is currently set by checking SharedPreferences.
82-111: LGTM! Switch change logic correctly handles enable/disable flows.The implementation properly:
- Shows confirmation dialog before enabling PIN protection
- Defers the switch state change until PIN creation succeeds (returns
falseat line 102)- Immediately removes PIN protection when disabled
- Provides clear warning messages about recovery via RPC
| private fun promptForPasswordVerification(correctPassword: String) { | ||
| val input = EditText(this) | ||
| input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD | ||
|
|
||
| AlertDialog.Builder(this) | ||
| .setTitle("Enter Pincode") | ||
| .setMessage("Enter your pincode to access settings:") | ||
| .setView(input) | ||
| .setPositiveButton("OK") { _, _ -> | ||
| val enteredPassword = input.text.toString() | ||
| if (enteredPassword == correctPassword) { | ||
| openSettings() | ||
| } else { | ||
| Toast.makeText(this, "Incorrect pincode", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| .setNegativeButton("Cancel") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .setCancelable(false) | ||
| .show() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add rate limiting to prevent brute-force attacks.
The PIN verification dialog has no rate limiting or maximum attempt restrictions. An attacker with physical access could repeatedly attempt PINs (especially if they're short) until finding the correct one.
Consider:
- Adding a delay after failed attempts (exponential backoff)
- Implementing a temporary lockout after N failed attempts
- Logging failed attempts
Additionally, note that setCancelable(false) at line 748 is redundant since a Cancel button is already provided at line 745.
🤖 Prompt for AI Agents
In @app/src/main/java/com/immichframe/immichframe/MainActivity.kt around lines
729 - 750, The PIN dialog in promptForPasswordVerification lacks rate limiting
and a max-attempt lockout and also uses a redundant setCancelable(false); fix by
adding an attempt counter and last-failure timestamp (e.g., in
SharedPreferences) keyed to the settings access, check these values before
verifying and refuse/show a locked message when the temporary lockout is active,
increment attempts on each wrong entry, compute an exponential backoff delay (or
fixed lock duration after N failures), persist the backoff/lockout state and
reset attempts on successful auth, disable the OK button (or ignore input) while
the backoff timer is active, log each failed attempt with timestamps (using Log
or existing analytics/logging helpers), and remove the redundant
setCancelable(false) call from the AlertDialog chain in
promptForPasswordVerification.
| private fun promptForPasswordCreation(chkSettingsPincode: SwitchPreferenceCompat?) { | ||
| val input = android.widget.EditText(requireContext()) | ||
| input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD | ||
|
|
||
| AlertDialog.Builder(requireContext()) | ||
| .setTitle("Create Pincode") | ||
| .setMessage("Enter a pincode to protect settings access:") | ||
| .setView(input) | ||
| .setPositiveButton("Set") { _, _ -> | ||
| val password = input.text.toString() | ||
| if (password.isEmpty()) { | ||
| Toast.makeText(requireContext(), "Pincode cannot be empty", Toast.LENGTH_SHORT).show() | ||
| } else { | ||
| // Save the password | ||
| val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext()) | ||
| sharedPrefs.edit() | ||
| .putString("settings_pincode", password) | ||
| .apply() | ||
|
|
||
| // Update the switch | ||
| chkSettingsPincode?.isChecked = true | ||
|
|
||
| Toast.makeText(requireContext(), "Pincode protection enabled", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| .setNegativeButton("Cancel") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .show() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce minimum PIN length for security.
The PIN creation only validates that it's not empty (line 190), allowing single-digit PINs that provide minimal security. Even against the stated use case (preventing children from changing settings), short PINs are easily guessable.
Recommend enforcing a minimum PIN length of 4-6 digits.
🔒 Proposed fix to enforce minimum PIN length
.setPositiveButton("Set") { _, _ ->
val password = input.text.toString()
- if (password.isEmpty()) {
- Toast.makeText(requireContext(), "Pincode cannot be empty", Toast.LENGTH_SHORT).show()
+ if (password.length < 4) {
+ Toast.makeText(requireContext(), "Pincode must be at least 4 digits", Toast.LENGTH_SHORT).show()
} else {
// Save the password
val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext())| @@ -0,0 +1,58 @@ | |||
| .PHONY: build clean install install-debug install-release test help | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing targets to .PHONY declaration.
The .PHONY declaration is incomplete. Several targets defined in the Makefile are missing: build-release, assemble, uninstall, lint, and run. Without declaring these as phony, Make may not execute them correctly if files with matching names exist.
📝 Proposed fix
-.PHONY: build clean install install-debug install-release test help
+.PHONY: help build build-release assemble clean install install-debug install-release uninstall test lint run📝 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.
| .PHONY: build clean install install-debug install-release test help | |
| .PHONY: help build build-release assemble clean install install-debug install-release uninstall test lint run |
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 1-1: Missing required phony target "all"
(minphony)
🤖 Prompt for AI Agents
In @Makefile at line 1, The .PHONY declaration is missing several targets so
Make may treat them as files; update the .PHONY line (the existing .PHONY: build
clean install install-debug install-release test help) to include the additional
phony targets build-release, assemble, uninstall, lint, and run so Make always
executes those rules instead of treating same‑named files as up‑to‑date.
Add the ability to block settings access behind a numerical passcode. This is perfect for protecting my frames from my inquisitve children.
curl http://$IP:53287/settingsstill works and bypasses the pin code. My kids aren't that smart (yet).Some notes:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.