@stealthcopter sorry for the ping, but I see you've been involved in #147, so you might be interested in reviewing this. Thanks!

Hey @Tachi107 Sorry it's taken so very long to respond. I have a window between jobs at the moment and attempting to address these PRs, update the documentation around the root checks and release a new version of rootbeer. In terms of this PR the checkForDangerousProps check is working as intended it'll flag for any of the properties not matching the expected value so changes in this PR are not aligned here so closing this PR. But thank you for your efforts here.

There's a different question in terms of whether the ro.debuggable is truly an indication of root. In one sense it's not, on the other it's an indication the device is not in a production-ready state so the plan is to keep the functionality as it for the time being.

To clarify @eksencncozay is not a maintainer of this project. Perhaps they can add some context to their review/comment/

Again sorry it's taken so long to respond.

Hi Scott, thanks for the reply! Don't worry about the time it took to respond, good things take time :)

I do agree that, looking back at this patch, only returning true if both properties are in an unexpected state is wrong. I am now of the opinion that the function should return true only if ro.secure is set to 0.

The purpose of checkForDangerousProps is to check for properties which might imply root access. As thoroughly discussed in #147, ro.secure likely implies root, while ro.debuggable is unrelated to this.

Yes, ro.debuggable might imply that the phone is in a non-production-ready state, but there's already an official way to check for "certified devices": SafetyNet, or more recently, Google Play Integrity API.

And as seen in #147, checking for ro.debuggable for the reason of checking non-production-readyness is flawed, as many devices are sold with the property enabled.

In short: a root checking library should only check for ro.secure, an integrity checking library cannot rely on any of them (and you can't beat Google's Play Integrity API in this)

Hope this makes sense to you! Thanks again and bye :)

In short: a root checking library should only check for ro.secure, an integrity checking library cannot rely on any of them (and you can't beat Google's Play Integrity API in this)

Thanks for the extra comments. Yes, agree with this. Given removing the ro.debuggable maybe something users are relying on I think it would be better placed to add this to new major version of the library.