Merge "Migrate PermissionAnnotationDetector to global lint checks" into main
diff --git a/services/accessibility/Android.bp b/services/accessibility/Android.bp
index b97ff62..9fd2f31 100644
--- a/services/accessibility/Android.bp
+++ b/services/accessibility/Android.bp
@@ -20,11 +20,6 @@
defaults: [
"platform_service_defaults",
],
- lint: {
- error_checks: ["MissingPermissionAnnotation"],
- baseline_filename: "lint-baseline.xml",
-
- },
srcs: [
":services.accessibility-sources",
"//frameworks/base/packages/SettingsLib/RestrictedLockUtils:SettingsLibRestrictedLockUtilsSrc",
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java
index 1b2447e..617cca9 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java
@@ -67,7 +67,6 @@
*
* NOTE: This class has to be created and poked only from the main thread.
*/
-@SuppressWarnings("MissingPermissionAnnotation")
class AccessibilityInputFilter extends InputFilter implements EventStreamTransformation {
private static final String TAG = AccessibilityInputFilter.class.getSimpleName();
diff --git a/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java b/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java
index e10e87c..c9ec16e 100644
--- a/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java
+++ b/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java
@@ -33,7 +33,6 @@
/**
* Encapsulate fingerprint gesture logic
*/
-@SuppressWarnings("MissingPermissionAnnotation")
public class FingerprintGestureDispatcher extends IFingerprintClientActiveCallback.Stub
implements Handler.Callback{
private static final int MSG_REGISTER = 1;
diff --git a/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
index f5af99e..b79563f 100644
--- a/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
+++ b/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
@@ -103,3 +103,26 @@
return fix.build()
}
+
+/**
+ * PermissionAnnotationDetector uses this method to determine whether a specific file should be
+ * checked for unannotated methods. Only files located in directories whose paths begin with one
+ * of these prefixes will be considered.
+ */
+fun isSystemServicePath(context: JavaContext): Boolean {
+ val systemServicePathPrefixes = setOf(
+ "frameworks/base/services",
+ "frameworks/base/apex",
+ "frameworks/opt/wear",
+ "packages/modules"
+ )
+
+ val filePath = context.file.path
+
+ // We perform `filePath.contains` instead of `filePath.startsWith` since getting the
+ // relative path of a source file is non-trivial. That is because `context.file.path`
+ // returns the path to where soong builds the file (i.e. /out/soong/...). Moreover, the
+ // logic to extract the relative path would need to consider several /out/soong/...
+ // locations patterns.
+ return systemServicePathPrefixes.any { filePath.contains(it) }
+}
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
index 5c64697..af753e5 100644
--- a/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
@@ -20,7 +20,6 @@
import com.android.tools.lint.client.api.Vendor
import com.android.tools.lint.detector.api.CURRENT_API
import com.google.android.lint.parcel.SaferParcelChecker
-import com.google.android.lint.aidl.PermissionAnnotationDetector
import com.google.auto.service.AutoService
@AutoService(IssueRegistry::class)
@@ -38,7 +37,6 @@
SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
// TODO: Currently crashes due to OOM issue
// PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
- PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,
PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,
FeatureAutomotiveDetector.ISSUE,
diff --git a/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt
deleted file mode 100644
index bce848a..0000000
--- a/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt
+++ /dev/null
@@ -1,134 +0,0 @@
-/*
- * Copyright (C) 2023 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.google.android.lint.aidl
-
-import com.android.tools.lint.checks.infrastructure.LintDetectorTest
-import com.android.tools.lint.checks.infrastructure.TestFile
-import com.android.tools.lint.checks.infrastructure.TestLintTask
-import com.android.tools.lint.detector.api.Detector
-import com.android.tools.lint.detector.api.Issue
-
-@Suppress("UnstableApiUsage")
-class PermissionAnnotationDetectorTest : LintDetectorTest() {
- override fun getDetector(): Detector = PermissionAnnotationDetector()
-
- override fun getIssues(): List<Issue> = listOf(
- PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
- )
-
- override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
-
- /** No issue scenario */
-
- fun testDoesNotDetectIssuesInCorrectScenario() {
- lint().files(
- java(
- """
- public class Foo extends IFoo.Stub {
- @Override
- @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
- public void testMethod() { }
- }
- """
- ).indented(),
- *stubs
- )
- .run()
- .expectClean()
- }
-
- fun testMissingAnnotation() {
- lint().files(
- java(
- """
- public class Bar extends IBar.Stub {
- public void testMethod() { }
- }
- """
- ).indented(),
- *stubs
- )
- .run()
- .expect(
- """
- src/Bar.java:2: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation]
- public void testMethod() { }
- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- 1 errors, 0 warnings
- """
- )
- }
-
- fun testNoIssueWhenExtendingWithAnotherSubclass() {
- lint().files(
- java(
- """
- public class Foo extends IFoo.Stub {
- @Override
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public void testMethod() { }
- // not an AIDL method, just another method
- public void someRandomMethod() { }
- }
- """).indented(),
- java(
- """
- public class Baz extends Bar {
- @Override
- public void someRandomMethod() { }
- }
- """).indented(),
- *stubs
- )
- .run()
- .expectClean()
- }
-
- /* Stubs */
-
- // A service with permission annotation on the method.
- private val interfaceIFoo: TestFile = java(
- """
- public interface IFoo extends android.os.IInterface {
- public static abstract class Stub extends android.os.Binder implements IFoo {
- }
- @Override
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public void testMethod();
- @Override
- @android.annotation.RequiresNoPermission
- public void testMethodNoPermission();
- @Override
- @android.annotation.PermissionManuallyEnforced
- public void testMethodManual();
- }
- """
- ).indented()
-
- // A service with no permission annotation.
- private val interfaceIBar: TestFile = java(
- """
- public interface IBar extends android.os.IInterface {
- public static abstract class Stub extends android.os.Binder implements IBar {
- }
- public void testMethod();
- }
- """
- ).indented()
-
- private val stubs = arrayOf(interfaceIFoo, interfaceIBar)
-}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
index 28eab8f..290e7be 100644
--- a/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
@@ -20,6 +20,7 @@
import com.android.tools.lint.client.api.Vendor
import com.android.tools.lint.detector.api.CURRENT_API
import com.google.android.lint.aidl.EnforcePermissionDetector
+import com.google.android.lint.aidl.PermissionAnnotationDetector
import com.google.android.lint.aidl.SimpleManualPermissionEnforcementDetector
import com.google.auto.service.AutoService
@@ -31,6 +32,7 @@
EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
EnforcePermissionDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
EnforcePermissionDetector.ISSUE_MISUSING_ENFORCE_PERMISSION,
+ PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
)
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt
index 8777712..675a59e6 100644
--- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt
@@ -20,12 +20,8 @@
* The exemptAidlInterfaces set was generated by running ExemptAidlInterfacesGenerator on the
* entire source tree. To reproduce the results, run generate-exempt-aidl-interfaces.sh
* located in tools/lint/utils.
- *
- * TODO: b/363248121 - Use the exemptAidlInterfaces set inside PermissionAnnotationDetector when it
- * gets migrated to a global lint check.
*/
val exemptAidlInterfaces = setOf(
- "android.accessibilityservice.IAccessibilityServiceConnection",
"android.accessibilityservice.IBrailleDisplayConnection",
"android.accounts.IAccountAuthenticatorResponse",
"android.accounts.IAccountManager",
@@ -663,10 +659,6 @@
"android.uwb.IUwbOemExtensionCallback",
"android.uwb.IUwbRangingCallbacks",
"android.uwb.IUwbVendorUciCallback",
- "android.view.accessibility.IAccessibilityInteractionConnectionCallback",
- "android.view.accessibility.IAccessibilityManager",
- "android.view.accessibility.IMagnificationConnectionCallback",
- "android.view.accessibility.IRemoteMagnificationAnimationCallback",
"android.view.autofill.IAutoFillManager",
"android.view.autofill.IAutofillWindowPresenter",
"android.view.contentcapture.IContentCaptureManager",
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/PermissionAnnotationDetector.kt
similarity index 92%
rename from tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt
rename to tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/PermissionAnnotationDetector.kt
index 6b50cfd..d44c271 100644
--- a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/PermissionAnnotationDetector.kt
@@ -43,8 +43,14 @@
interfaceName: String,
body: UBlockExpression
) {
+ if (!isSystemServicePath(context)) return
+
if (context.evaluator.isAbstract(node)) return
+ val fullyQualifiedInterfaceName =
+ getContainingAidlInterfaceQualified(context, node) ?: return
+ if (exemptAidlInterfaces.contains(fullyQualifiedInterfaceName)) return
+
if (AIDL_PERMISSION_ANNOTATIONS.any { node.hasAnnotation(it) }) return
context.report(
@@ -80,8 +86,7 @@
implementation = Implementation(
PermissionAnnotationDetector::class.java,
Scope.JAVA_FILE_SCOPE
- ),
- enabledByDefault = false
+ )
)
}
}
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/PermissionAnnotationDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/PermissionAnnotationDetectorTest.kt
new file mode 100644
index 0000000..92d0829
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/PermissionAnnotationDetectorTest.kt
@@ -0,0 +1,230 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.lint.aidl
+
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.checks.infrastructure.TestFile
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+
+@Suppress("UnstableApiUsage")
+class PermissionAnnotationDetectorTest : LintDetectorTest() {
+ override fun getDetector(): Detector =
+ PermissionAnnotationDetector()
+
+ override fun getIssues(): List<Issue> = listOf(
+ PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+ /** No issue scenario */
+
+ fun testDoesNotDetectIssuesInCorrectScenario() {
+ lint()
+ .files(
+ java(
+ createVisitedPath("Foo.java"),
+ """
+ package com.android.server;
+ public class Foo extends IFoo.Stub {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void testMethod() { }
+ }
+ """
+ )
+ .indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testMissingAnnotation() {
+ lint()
+ .files(
+ java(
+ createVisitedPath("Bar.java"),
+ """
+ package com.android.server;
+ public class Bar extends IBar.Stub {
+ public void testMethod() { }
+ }
+ """
+ )
+ .indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/frameworks/base/services/java/com/android/server/Bar.java:3: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation]
+ public void testMethod() { }
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """
+ )
+ }
+
+ fun testMissingAnnotationInIgnoredDirectory() {
+ lint()
+ .files(
+ java(
+ ignoredPath,
+ """
+ package com.android.server;
+ public class Bar extends IBar.Stub {
+ public void testMethod() { }
+ }
+ """
+ )
+ .indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ // If this test fails, consider the following steps:
+ // 1. Pick the first entry (interface) from `exemptAidlInterfaces`.
+ // 2. Change `interfaceIExempted` to use that interface.
+ // 3. Change this test's class to extend the interface's Stub.
+ fun testMissingAnnotationAidlInterfaceExempted() {
+ lint()
+ .files(
+ java(
+ createVisitedPath("Bar.java"),
+ """
+ package com.android.server;
+ public class Bar extends android.accessibilityservice.IBrailleDisplayConnection.Stub {
+ public void testMethod() { }
+ }
+ """
+ )
+ .indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testMissingAnnotationAidlInterfaceAbstractMethod() {
+ lint()
+ .files(
+ java(
+ createVisitedPath("Bar.java"),
+ """
+ package com.android.server;
+ public abstract class Bar extends IBar.Stub {
+ public abstract void testMethod();
+ }
+ """
+ )
+ .indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testNoIssueWhenExtendingWithAnotherSubclass() {
+ lint()
+ .files(
+ java(
+ createVisitedPath("Foo.java"),
+ """
+ package com.android.server;
+ public class Foo extends IFoo.Stub {
+ @Override
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void testMethod() { }
+ // not an AIDL method, just another method
+ public void someRandomMethod() { }
+ }
+ """
+ )
+ .indented(),
+ java(
+ createVisitedPath("Baz.java"),
+ """
+ package com.android.server;
+ public class Baz extends Bar {
+ @Override
+ public void someRandomMethod() { }
+ }
+ """
+ )
+ .indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ /* Stubs */
+
+ // A service with permission annotation on the method.
+ private val interfaceIFoo: TestFile = java(
+ """
+ public interface IFoo extends android.os.IInterface {
+ public static abstract class Stub extends android.os.Binder implements IFoo {
+ }
+ @Override
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void testMethod();
+ @Override
+ @android.annotation.RequiresNoPermission
+ public void testMethodNoPermission();
+ @Override
+ @android.annotation.PermissionManuallyEnforced
+ public void testMethodManual();
+ }
+ """
+ ).indented()
+
+ // A service with no permission annotation.
+ private val interfaceIBar: TestFile = java(
+ """
+ public interface IBar extends android.os.IInterface {
+ public static abstract class Stub extends android.os.Binder implements IBar {
+ }
+ public void testMethod();
+ }
+ """
+ ).indented()
+
+ // A service whose AIDL Interface is exempted.
+ private val interfaceIExempted: TestFile = java(
+ """
+ package android.accessibilityservice;
+ public interface IBrailleDisplayConnection extends android.os.IInterface {
+ public static abstract class Stub extends android.os.Binder implements IBrailleDisplayConnection {
+ }
+ public void testMethod();
+ }
+ """
+ ).indented()
+
+ private val stubs = arrayOf(interfaceIFoo, interfaceIBar, interfaceIExempted)
+
+ private fun createVisitedPath(filename: String) =
+ "src/frameworks/base/services/java/com/android/server/$filename"
+
+ private val ignoredPath = "src/test/pkg/TestClass.java"
+}
diff --git a/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt b/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt
index 6ad223c..57c2e5a 100644
--- a/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt
+++ b/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt
@@ -33,12 +33,6 @@
*/
class ExemptAidlInterfacesGenerator : AidlImplementationDetector() {
private val targetExemptAidlInterfaceNames = mutableSetOf<String>()
- private val systemServicePathPrefixes = setOf(
- "frameworks/base/services",
- "frameworks/base/apex",
- "frameworks/opt/wear",
- "packages/modules"
- )
// We could've improved performance by visiting classes rather than methods, however, this lint
// check won't be run regularly, hence we've decided not to add extra overrides to
@@ -49,14 +43,7 @@
interfaceName: String,
body: UBlockExpression
) {
- val filePath = context.file.path
-
- // We perform `filePath.contains` instead of `filePath.startsWith` since getting the
- // relative path of a source file is non-trivial. That is because `context.file.path`
- // returns the path to where soong builds the file (i.e. /out/soong/...). Moreover, the
- // logic to extract the relative path would need to consider several /out/soong/...
- // locations patterns.
- if (systemServicePathPrefixes.none { filePath.contains(it) }) return
+ if (!isSystemServicePath(context)) return
val fullyQualifiedInterfaceName =
getContainingAidlInterfaceQualified(context, node) ?: return
