summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMakoto Onuki <omakoto@google.com>2020-06-12 01:41:15 +0000
committerAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>2020-06-12 01:41:15 +0000
commit8b485239dcf3c54e63dce24c19c6a8c4d8aa6603 (patch)
tree0e1de2878c86394142242b09ae20498a74f4a04b
parentc1963f8898b3f714f475363ba068716cc757e139 (diff)
parent1725ea84ae9e9e852f582df0072e2c661f8cc51a (diff)
[Metalava] Add "strict input" mode am: 1725ea84ae
Original change: https://googleplex-android-review.googlesource.com/c/platform/tools/metalava/+/11832628 Change-Id: Iff527da0b6db035891f40da86728b13102ec2233
-rw-r--r--src/main/java/com/android/tools/metalava/Driver.kt45
-rw-r--r--src/main/java/com/android/tools/metalava/FileReadSandbox.kt315
-rw-r--r--src/main/java/com/android/tools/metalava/Options.kt153
-rw-r--r--src/test/java/com/android/tools/metalava/FileReadSandboxTest.kt159
-rw-r--r--src/test/java/com/android/tools/metalava/OptionsTest.kt46
5 files changed, 687 insertions, 31 deletions
diff --git a/src/main/java/com/android/tools/metalava/Driver.kt b/src/main/java/com/android/tools/metalava/Driver.kt
index 8de2bc6..9626d7f 100644
--- a/src/main/java/com/android/tools/metalava/Driver.kt
+++ b/src/main/java/com/android/tools/metalava/Driver.kt
@@ -86,6 +86,8 @@ fun main(args: Array<String>) {
run(args, setExitCode = true)
}
+internal var hasFileReadViolations = false
+
/**
* The metadata driver is a command line interface to extracting various metadata
* from a source tree (or existing signature files etc). Run with --help to see
@@ -111,11 +113,17 @@ fun run(
compatibility = Compatibility(compat = Options.useCompatMode(modifiedArgs))
options = Options(modifiedArgs, stdout, stderr)
+ maybeActivateSandbox()
+
processFlags()
if (options.allReporters.any { it.hasErrors() } && !options.passBaselineUpdates) {
exitCode = -1
}
+ if (hasFileReadViolations) {
+ stderr.println("$PROGRAM_NAME detected access to files that are not explicitly specified. See ${options.strictInputViolationsFile} for details.")
+ exitCode = -1
+ }
} catch (e: DriverException) {
stdout.flush()
stderr.flush()
@@ -143,6 +151,7 @@ fun run(
}
options.reportEvenIfSuppressedWriter?.close()
+ options.strictInputViolationsPrintWriter?.close()
// Show failure messages, if any.
options.allReporters.forEach {
@@ -168,6 +177,35 @@ private fun exit(exitCode: Int = 0) {
System.exit(exitCode)
}
+private fun maybeActivateSandbox() {
+ // Set up a sandbox to detect access to files that are not explicitly specified.
+ if (options.strictInputFiles == Options.StrictInputFileMode.PERMISSIVE) {
+ return
+ }
+
+ val writer = options.strictInputViolationsPrintWriter!!
+
+ // Writes all violations to [Options.strictInputFiles].
+ // Note violation reads on directories are logged, but is considered to be a "warning" and
+ // doesn't affect the exit code. See [FileReadSandbox] for the details.
+ FileReadSandbox.activate(object : FileReadSandbox.Listener {
+ var seen = mutableSetOf<String>()
+ override fun onViolation(absolutePath: String, isDirectory: Boolean) {
+ if (!seen.contains(absolutePath)) {
+ val suffix = if (isDirectory) "/" else ""
+ writer.println("Sandbox violation: $absolutePath$suffix")
+ if (options.strictInputFiles == Options.StrictInputFileMode.STRICT_WITH_STACK) {
+ Throwable().printStackTrace(writer)
+ }
+ seen.add(absolutePath)
+ if (!isDirectory) {
+ hasFileReadViolations = true
+ }
+ }
+ }
+ })
+}
+
private fun processFlags() {
val stopwatch = Stopwatch.createStarted()
@@ -863,10 +901,13 @@ internal fun parseSources(
val joined = mutableListOf<File>()
joined.addAll(sourcePath.mapNotNull { if (it.path.isNotBlank()) it.absoluteFile else null })
joined.addAll(classpath.map { it.absoluteFile })
+
// Add in source roots implied by the source files
val sourceRoots = mutableListOf<File>()
- extractRoots(sources, sourceRoots)
- joined.addAll(sourceRoots)
+ if (options.allowImplicitRoot) {
+ extractRoots(sources, sourceRoots)
+ joined.addAll(sourceRoots)
+ }
// Create project environment with those paths
projectEnvironment.registerPaths(joined)
diff --git a/src/main/java/com/android/tools/metalava/FileReadSandbox.kt b/src/main/java/com/android/tools/metalava/FileReadSandbox.kt
new file mode 100644
index 0000000..02bbae2
--- /dev/null
+++ b/src/main/java/com/android/tools/metalava/FileReadSandbox.kt
@@ -0,0 +1,315 @@
+/*
+ * Copyright (C) 2020 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.android.tools.metalava
+
+import java.io.File
+import java.io.FileDescriptor
+import java.net.InetAddress
+import java.security.Permission
+import kotlin.concurrent.getOrSet
+
+/**
+ * Detect access to files that not explicitly specified in the command line.
+ *
+ * This class detects reads on both files and directories. Directory accesses are logged by
+ * [Driver], which only logs it, but doesn't consider it an error.
+ *
+ * We do not prevent reads on directories that are not explicitly listed in the command line because
+ * metalava (or JVM, really) seems to read some system directories such as /usr/, etc., but this
+ * behavior may be JVM dependent so we do not want to have to explicitly whitelist them.
+ * (Because, otherwise, when we update JVM, it may access different directories and we end up
+ * having to update the implicit whitelist.) As long as we don't read files, reading directories
+ * shouldn't (normally) affect the result, so we simply allow any directory reads.
+ */
+internal object FileReadSandbox {
+ private var installed = false
+
+ private var previousSecurityManager: SecurityManager? = null
+ private val mySecurityManager = MySecurityManager()
+
+ /**
+ * Set of paths that are allowed to access. This is used with an exact match.
+ */
+ private var allowedPaths = mutableSetOf<String>()
+
+ /**
+ * Set of path prefixes that are allowed to access.
+ */
+ private var allowedPathPrefixes = mutableSetOf<String>()
+
+ interface Listener {
+ /** Called when a violation is detected with the absolute path. */
+ fun onViolation(absolutePath: String, isDirectory: Boolean)
+ }
+
+ private var listener: Listener? = null
+
+ init {
+ initialize()
+ }
+
+ private fun initialize() {
+ allowedPaths.clear()
+ allowedPathPrefixes.clear()
+ listener = null
+
+ // Allow certain directories by default.
+ // The sandbox detects all file accessed done by JVM implicitly too (e.g. JVM seems to
+ // access /dev/urandom on linux), so we need to allow them.
+ // At least on linux, there doesn't seem to be any access to files under /proc/ or /sys/,
+ // but they should be allowed anyway.
+ listOf("/dev", "/proc", "/sys").forEach {
+ allowAccess(File(it))
+ }
+ // Allow access to the directory containing the metalava's jar itself.
+ // (so even if metalava loads other jar files in the same directory, that wouldn't be a
+ // violation.)
+ FileReadSandbox::class.java.protectionDomain?.codeSource?.location?.toURI()?.path?.let {
+ allowAccess(File(it).parentFile)
+ }
+
+ // Allow access to $JAVA_HOME.
+ // We also allow $ANDROID_JAVA_HOME, which is used in the Android platform build.
+ // (which is normally $ANDROID_BUILD_TOP + "/prebuilts/jdk/jdk11/linux-x86" as of writing.)
+ listOf(
+ "JAVA_HOME",
+ "ANDROID_JAVA_HOME"
+ ).forEach {
+ System.getenv(it)?.let {
+ allowAccess(File(it))
+ }
+ }
+ // JVM seems to use ~/.cache/
+ System.getenv("HOME")?.let {
+ allowAccess(File("$it/.cache"))
+ }
+ }
+
+ /** Activate the sandbox. */
+ fun activate(listener: Listener) {
+ if (installed) {
+ throw IllegalStateException("Already activated")
+ }
+ previousSecurityManager = System.getSecurityManager()
+ System.setSecurityManager(mySecurityManager)
+ installed = true
+ this.listener = listener
+ }
+
+ /** Deactivate the sandbox. This also resets [violationCount]. */
+ fun deactivate() {
+ if (!installed) {
+ throw IllegalStateException("Not activated")
+ }
+ System.setSecurityManager(previousSecurityManager)
+ previousSecurityManager = null
+
+ installed = false
+ }
+
+ /** Reset all the state and re-initialize the sandbox. Only callable when not activated. */
+ fun reset() {
+ if (installed) {
+ throw IllegalStateException("Can't reset. Already activated")
+ }
+ initialize()
+ }
+
+ /** Allows access to files or directories. */
+ fun allowAccess(files: List<File>): List<File> {
+ files.forEach { allowAccess(it) }
+ return files
+ }
+
+ /** Allows access to a file or directory. */
+ fun <T : File?> allowAccess(file: T): T {
+ if (file == null) {
+ return file
+ }
+ val path = file.absolutePath
+ val added = allowedPaths.add(path)
+ if (file.isDirectory) {
+ // Even if it had already been added to allowedPaths (i.e. added == false),
+ // the directory might have just been created, so we still do it.
+ allowedPathPrefixes.add("$path/")
+ }
+ if (!added) {
+ // Already added; bail early.
+ return file
+ }
+
+ // Whitelist all parent directories. But don't allow prefix accesses (== access to the
+ // directory itself is okay, but don't grant access to any files/directories under it).
+ var parent = file.parentFile
+ while (true) {
+ if (parent == null) {
+ break
+ }
+ @Suppress("NAME_SHADOWING")
+ val path = parent.absolutePath
+ if (!allowedPaths.add(path)) {
+ break // already added.
+ }
+ if (path == "/") {
+ break
+ }
+ parent = parent.parentFile
+ }
+ return file
+ }
+
+ fun isAccessAllowed(file: File): Boolean {
+ val absPath = file.absolutePath
+
+ if (allowedPaths.contains(absPath)) {
+ return true
+ }
+ allowedPathPrefixes.forEach {
+ if (absPath.startsWith(it)) {
+ return true
+ }
+ }
+ return false
+ }
+
+ /** Used to skip all checks on any filesystem access made within the [check] method. */
+ private val temporaryExempt = ThreadLocal<Boolean>()
+
+ private fun check(path: String) {
+ if (temporaryExempt.getOrSet { false }) {
+ return
+ }
+
+ try {
+ temporaryExempt.set(true)
+
+ val file = File(path)
+
+ if (!isAccessAllowed(file)) {
+ listener?.onViolation(file.absolutePath, file.isDirectory)
+ }
+ } finally {
+ temporaryExempt.set(false)
+ }
+ }
+
+ /**
+ * Reading files that are created by metalava should be allowed, so we detect file writes to
+ * new files, and whitelist it.
+ */
+ private fun writeDetected(origPath: String?) {
+ if (temporaryExempt.getOrSet { false }) {
+ return
+ }
+
+ try {
+ temporaryExempt.set(true)
+
+ val file = File(origPath)
+ if (file.exists()) {
+ return // Ignore writes to an existing file / directory.
+ }
+ allowedPaths.add(file.absolutePath)
+ } finally {
+ temporaryExempt.set(false)
+ }
+ }
+
+ private class MySecurityManager : SecurityManager() {
+ override fun checkRead(file: String) {
+ check(file)
+ }
+
+ override fun checkRead(file: String, context: Any?) {
+ check(file)
+ }
+ override fun checkRead(p0: FileDescriptor?) {
+ }
+
+ override fun checkDelete(p0: String?) {
+ }
+
+ override fun checkPropertiesAccess() {
+ }
+
+ override fun checkAccess(p0: Thread?) {
+ }
+
+ override fun checkAccess(p0: ThreadGroup?) {
+ }
+
+ override fun checkExec(p0: String?) {
+ }
+
+ override fun checkListen(p0: Int) {
+ }
+
+ override fun checkExit(p0: Int) {
+ }
+
+ override fun checkLink(p0: String?) {
+ }
+
+ override fun checkPropertyAccess(p0: String?) {
+ }
+
+ override fun checkPackageDefinition(p0: String?) {
+ }
+
+ override fun checkMulticast(p0: InetAddress?) {
+ }
+
+ override fun checkMulticast(p0: InetAddress?, p1: Byte) {
+ }
+
+ override fun checkPermission(p0: Permission?) {
+ }
+
+ override fun checkPermission(p0: Permission?, p1: Any?) {
+ }
+
+ override fun checkPackageAccess(p0: String?) {
+ }
+
+ override fun checkAccept(p0: String?, p1: Int) {
+ }
+
+ override fun checkSecurityAccess(p0: String?) {
+ }
+
+ override fun checkWrite(p0: FileDescriptor?) {
+ }
+
+ override fun checkWrite(file: String?) {
+ writeDetected(file)
+ }
+
+ override fun checkPrintJobAccess() {
+ }
+
+ override fun checkCreateClassLoader() {
+ }
+
+ override fun checkConnect(p0: String?, p1: Int) {
+ }
+
+ override fun checkConnect(p0: String?, p1: Int, p2: Any?) {
+ }
+
+ override fun checkSetFactory() {
+ }
+ }
+} \ No newline at end of file
diff --git a/src/main/java/com/android/tools/metalava/Options.kt b/src/main/java/com/android/tools/metalava/Options.kt
index 5ffa7ce..7e0a888 100644
--- a/src/main/java/com/android/tools/metalava/Options.kt
+++ b/src/main/java/com/android/tools/metalava/Options.kt
@@ -166,6 +166,10 @@ const val ARG_IGNORE_CLASSES_ON_CLASSPATH = "--ignore-classes-on-classpath"
const val ARG_ERROR_MESSAGE_API_LINT = "--error-message:api-lint"
const val ARG_ERROR_MESSAGE_CHECK_COMPATIBILITY_RELEASED = "--error-message:compatibility:released"
const val ARG_ERROR_MESSAGE_CHECK_COMPATIBILITY_CURRENT = "--error-message:compatibility:current"
+const val ARG_NO_IMPLICIT_ROOT = "--no-implicit-root"
+const val ARG_STRICT_INPUT_FILES = "--strict-input-files"
+const val ARG_STRICT_INPUT_FILES_STACK = "--strict-input-files:stack"
+const val ARG_STRICT_INPUT_FILES_EXEMPT = "--strict-input-files-exempt"
class Options(
private val args: Array<String>,
@@ -657,6 +661,34 @@ class Options(
/** How to handle typedef annotations in signature files; corresponds to $ARG_TYPEDEFS_IN_SIGNATURES */
var typedefMode = TypedefMode.NONE
+ /** Allow implicit root detection (which is the default behavior). See [ARG_NO_IMPLICIT_ROOT] */
+ var allowImplicitRoot = true
+
+ enum class StrictInputFileMode {
+ PERMISSIVE,
+ STRICT,
+ STRICT_WITH_STACK;
+
+ companion object {
+ fun fromArgument(arg: String): StrictInputFileMode {
+ return when (arg) {
+ ARG_STRICT_INPUT_FILES -> STRICT
+ ARG_STRICT_INPUT_FILES_STACK -> STRICT_WITH_STACK
+ else -> PERMISSIVE
+ }
+ }
+ }
+ }
+
+ /**
+ * Whether we should allow metalava to read files that are not explicitly specified in the
+ * command line. See [ARG_STRICT_INPUT_FILES] and [ARG_STRICT_INPUT_FILES_STACK]
+ */
+ var strictInputFiles = StrictInputFileMode.PERMISSIVE
+
+ var strictInputViolationsFile: File? = null
+ var strictInputViolationsPrintWriter: PrintWriter? = null
+
/** File conversion tasks */
data class ConvertFile(
val fromApiFile: File,
@@ -974,7 +1006,7 @@ class Options(
if (index < args.size - 1) {
val nextArg = args[index + 1]
if (!nextArg.startsWith("-")) {
- val file = fileForPath(nextArg)
+ val file = stringToExistingFile(nextArg)
if (file.isFile) {
index++
migrateNullsFrom = file
@@ -1005,7 +1037,7 @@ class Options(
if (index < args.size - 1) {
val nextArg = args[index + 1]
if (!nextArg.startsWith("-")) {
- val file = fileForPath(nextArg)
+ val file = stringToExistingFile(nextArg)
if (file.isFile) {
index++
mutableCompatibilityChecks.add(CheckRequest(file, ApiType.PUBLIC_API, ReleaseType.DEV))
@@ -1297,6 +1329,28 @@ class Options(
}
}
+ ARG_NO_IMPLICIT_ROOT -> {
+ allowImplicitRoot = false
+ }
+
+ ARG_STRICT_INPUT_FILES, ARG_STRICT_INPUT_FILES_STACK -> {
+ if (strictInputViolationsFile != null) {
+ throw DriverException("$ARG_STRICT_INPUT_FILES and $ARG_STRICT_INPUT_FILES_STACK may be specified only once")
+ }
+ strictInputFiles = StrictInputFileMode.fromArgument(arg)
+
+ val file = stringToNewOrExistingFile(getValue(args, ++index))
+ strictInputViolationsFile = file
+ strictInputViolationsPrintWriter = file.printWriter()
+ }
+ ARG_STRICT_INPUT_FILES_EXEMPT -> {
+ val listString = getValue(args, ++index)
+ listString.split(File.pathSeparatorChar).forEach { path ->
+ // Throw away the result; just let the function add the files to the whitelist.
+ stringToExistingFilesOrDirs(path)
+ }
+ }
+
"--temp-folder" -> {
tempFolder = stringToNewOrExistingDir(getValue(args, ++index))
}
@@ -1689,6 +1743,10 @@ class Options(
* Produce a default file name for the baseline. It's normally "baseline.txt", but can
* be prefixed by show annotations; e.g. @TestApi -> test-baseline.txt, @SystemApi -> system-baseline.txt,
* etc.
+ *
+ * Note because the default baseline file is not explicitly set in the command line,
+ * this file would trigger a --strict-input-files violation. To avoid that, always explicitly
+ * pass a baseline file.
*/
private fun getDefaultBaselineFile(): File? {
if (sourcePath.isNotEmpty() && sourcePath[0].path.isNotBlank()) {
@@ -1712,6 +1770,13 @@ class Options(
}
}
+ /**
+ * Find an android stub jar that matches the given criteria.
+ *
+ * Note because the default baseline file is not explicitly set in the command line,
+ * this file would trigger a --strict-input-files violation. To avoid that, use
+ * --strict-input-files-exempt to exempt the jar directory.
+ */
private fun findAndroidJars(
androidJarPatterns: List<String>,
currentApiLevel: Int,
@@ -1774,8 +1839,9 @@ class Options(
}
private fun getAndroidJarFile(apiLevel: Int, patterns: List<String>): File? {
+ // Note this method doesn't register the result to [FileReadSandbox]
return patterns
- .map { fileForPath(it.replace("%", Integer.toString(apiLevel))) }
+ .map { fileForPathInner(it.replace("%", Integer.toString(apiLevel))) }
.firstOrNull { it.isFile }
}
@@ -1854,78 +1920,86 @@ class Options(
}
private fun stringToExistingDir(value: String): File {
- val file = fileForPath(value)
+ val file = fileForPathInner(value)
if (!file.isDirectory) {
throw DriverException("$file is not a directory")
}
- return file
+ return FileReadSandbox.allowAccess(file)
}
@Suppress("unused")
private fun stringToExistingDirs(value: String): List<File> {
val files = mutableListOf<File>()
for (path in value.split(File.pathSeparatorChar)) {
- val file = fileForPath(path)
+ val file = fileForPathInner(path)
if (!file.isDirectory) {
throw DriverException("$file is not a directory")
}
files.add(file)
}
- return files
+ return FileReadSandbox.allowAccess(files)
}
private fun stringToExistingDirsOrJars(value: String): List<File> {
val files = mutableListOf<File>()
for (path in value.split(File.pathSeparatorChar)) {
- val file = fileForPath(path)
+ val file = fileForPathInner(path)
if (!file.isDirectory && !(file.path.endsWith(SdkConstants.DOT_JAR) && file.isFile)) {
throw DriverException("$file is not a jar or directory")
}
files.add(file)
}
- return files
+ return FileReadSandbox.allowAccess(files)
}
private fun stringToExistingDirsOrFiles(value: String): List<File> {
val files = mutableListOf<File>()
for (path in value.split(File.pathSeparatorChar)) {
- val file = fileForPath(path)
+ val file = fileForPathInner(path)
if (!file.exists()) {
throw DriverException("$file does not exist")
}
files.add(file)
}
- return files
+ return FileReadSandbox.allowAccess(files)
}
private fun stringToExistingFile(value: String): File {
- val file = fileForPath(value)
+ val file = fileForPathInner(value)
if (!file.isFile) {
throw DriverException("$file is not a file")
}
- return file
+ return FileReadSandbox.allowAccess(file)
}
@Suppress("unused")
private fun stringToExistingFileOrDir(value: String): File {
- val file = fileForPath(value)
+ val file = fileForPathInner(value)
if (!file.exists()) {
throw DriverException("$file is not a file or directory")
}
- return file
+ return FileReadSandbox.allowAccess(file)
}
private fun stringToExistingFiles(value: String): List<File> {
+ return stringToExistingFilesOrDirsInternal(value, false)
+ }
+
+ private fun stringToExistingFilesOrDirs(value: String): List<File> {
+ return stringToExistingFilesOrDirsInternal(value, true)
+ }
+
+ private fun stringToExistingFilesOrDirsInternal(value: String, allowDirs: Boolean): List<File> {
val files = mutableListOf<File>()
value.split(File.pathSeparatorChar)
- .map { fileForPath(it) }
+ .map { fileForPathInner(it) }
.forEach { file ->
if (file.path.startsWith("@")) {
// File list; files to be read are stored inside. SHOULD have been one per line
// but sadly often uses spaces for separation too (so we split by whitespace,
// which means you can't point to files in paths with spaces)
val listFile = File(file.path.substring(1))
- if (!listFile.isFile) {
+ if (!allowDirs && !listFile.isFile) {
throw DriverException("$listFile is not a file")
}
val contents = Files.asCharSource(listFile, UTF_8).read()
@@ -1933,23 +2007,23 @@ class Options(
contents
)
pathList.asSequence().map { File(it) }.forEach {
- if (!it.isFile) {
+ if (!allowDirs && !it.isFile) {
throw DriverException("$it is not a file")
}
files.add(it)
}
} else {
- if (!file.isFile) {
+ if (!allowDirs && !file.isFile) {
throw DriverException("$file is not a file")
}
files.add(file)
}
}
- return files
+ return FileReadSandbox.allowAccess(files)
}
private fun stringToNewFile(value: String): File {
- val output = fileForPath(value)
+ val output = fileForPathInner(value)
if (output.exists()) {
if (output.isDirectory) {
@@ -1966,22 +2040,22 @@ class Options(
}
}
- return output
+ return FileReadSandbox.allowAccess(output)
}
private fun stringToNewOrExistingDir(value: String): File {
- val dir = fileForPath(value)
+ val dir = fileForPathInner(value)
if (!dir.isDirectory) {
val ok = dir.mkdirs()
if (!ok) {
throw DriverException("Could not create $dir")
}
}
- return dir
+ return FileReadSandbox.allowAccess(dir)
}
private fun stringToNewOrExistingFile(value: String): File {
- val file = fileForPath(value)
+ val file = fileForPathInner(value)
if (!file.exists()) {
val parentFile = file.parentFile
if (parentFile != null && !parentFile.isDirectory) {
@@ -1991,11 +2065,11 @@ class Options(
}
}
}
- return file
+ return FileReadSandbox.allowAccess(file)
}
private fun stringToNewDir(value: String): File {
- val output = fileForPath(value)
+ val output = fileForPathInner(value)
val ok =
if (output.exists()) {
if (output.isDirectory) {
@@ -2013,10 +2087,19 @@ class Options(
throw DriverException("Could not create $output")
}
- return output
+ return FileReadSandbox.allowAccess(output)
}
- private fun fileForPath(path: String): File {
+ /**
+ * Converts a path to a [File] that represents the absolute path, with the following special
+ * behavior:
+ * - "~" will be expanded into the home directory path.
+ * - If the given path starts with "@", it'll be converted into "@" + [file's absolute path]
+ *
+ * Note, unlike the other "stringToXxx" methods, this method won't register the given path
+ * to [FileReadSandbox].
+ */
+ private fun fileForPathInner(path: String): File {
// java.io.File doesn't automatically handle ~/ -> home directory expansion.
// This isn't necessary when metalava is run via the command line driver
// (since shells will perform this expansion) but when metalava is run
@@ -2310,6 +2393,18 @@ class Options(
ARG_CURRENT_CODENAME, "Sets the code name for the current source code",
ARG_CURRENT_JAR, "Points to the current API jar, if any",
+ "", "\nSandboxing:",
+ ARG_NO_IMPLICIT_ROOT, "Disable implicit root directory detection. " +
+ "Otherwise, $PROGRAM_NAME adds in source roots implied by the source files",
+ "$ARG_STRICT_INPUT_FILES <file>", "Do not read files that are not explicitly specified in the command line. " +
+ "All violations are written to the given file. Reads on directories are always allowed, but " +
+ "$PROGRAM_NAME still trackes reads on directories that are not specified in the command line, " +
+ "and write them to the file.",
+ "$ARG_STRICT_INPUT_FILES_STACK <file>", "Same as $ARG_STRICT_INPUT_FILES but also print stacktraces.",
+ "$ARG_STRICT_INPUT_FILES_EXEMPT <files or dirs>", "Used with $ARG_STRICT_INPUT_FILES. Explicitly allow " +
+ "access to files and/or directories (separated by `${File.pathSeparator}). Can also be " +
+ "@ followed by a path to a text file containing paths to the full set of files and/or directories.",
+
"", "\nEnvironment Variables:",
ENV_VAR_METALAVA_DUMP_ARGV, "Set to true to have metalava emit all the arguments it was invoked with. " +
"Helpful when debugging or reproducing under a debugger what the build system is doing.",
diff --git a/src/test/java/com/android/tools/metalava/FileReadSandboxTest.kt b/src/test/java/com/android/tools/metalava/FileReadSandboxTest.kt
new file mode 100644
index 0000000..8341dfd
--- /dev/null
+++ b/src/test/java/com/android/tools/metalava/FileReadSandboxTest.kt
@@ -0,0 +1,159 @@
+/*
+ * Copyright (C) 2020 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.android.tools.metalava
+
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import java.io.File
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class FileReadSandboxTest {
+ @get:Rule
+ var temporaryFolder = TemporaryFolder()
+
+ @Test
+ fun `Test sandbox`() {
+ // Create target files.
+ val root = temporaryFolder.newFolder()
+
+ val goodFile = File(root, "goodFile").apply { createNewFile() }
+ val badFile = File(root, "badFile").apply { createNewFile() }
+
+ val goodDir = File(root, "goodDir").apply { mkdirs() }
+ val goodDirFile = File(goodDir, "file").apply { createNewFile() }
+
+ val badDir = File(root, "badDir").apply { mkdirs() }
+ val badDirFile = File(badDir, "file").apply { createNewFile() }
+
+ val subDir = File(root, "subdir").apply { mkdirs() }
+ val subSubDir = File(subDir, "subdir").apply { mkdirs() }
+ val subSubDir2 = File(subDir, "subdir2").apply { mkdirs() }
+ val subSubDirGoodFile = File(subSubDir, "good").apply { createNewFile() }
+ val subSubDirBadFile = File(subSubDir, "bad").apply { createNewFile() }
+
+ // Structure:
+ // - * Explicitly allowed
+ // - ** Implicitly allowed (parent dirctories of whitelisted paths)
+ // - @ Not allowed
+ // root **
+ // |-goodFile *
+ // |
+ // |-badFile @
+ // |
+ // |-goodDir *
+ // | |-goodDirFile **
+ // |
+ // |-badDir @
+ // | |-badDirFile @
+ // |
+ // |-subDir **
+ // |-subSubDir **
+ // | |-subSubDirGoodFile *
+ // | |-subSubDirBadFile @
+ // |-subSubDir2 @
+
+ // Set up the sandbox.
+ FileReadSandbox.allowAccess(goodFile)
+ FileReadSandbox.allowAccess(goodDir)
+ FileReadSandbox.allowAccess(subSubDirGoodFile)
+
+ var allowedSet = mutableSetOf(root, goodFile, goodDir, goodDirFile, subDir, subSubDir, subSubDirGoodFile).map { it.absolutePath }
+ var emptySet = setOf<String>()
+
+ var violations = mutableSetOf<String>()
+
+ // Make sure whitelisted files are not in the violation list.
+ fun assertWhitelistedFilesNotReported() {
+ assertEquals(emptySet, violations.intersect(allowedSet))
+ }
+
+ // Assert given files are reported as violations.
+ fun assertViolationReported(vararg files: File) {
+ val fileSet = files.map { it.absolutePath }.toSet()
+ assertEquals(fileSet, violations.intersect(fileSet))
+ }
+
+ // Assert given files are *not* reported as violations.
+ fun assertViolationNotReported(vararg files: File) {
+ val fileSet = files.map { it.absolutePath }.toSet()
+ assertEquals(emptySet, violations.intersect(fileSet))
+ }
+
+ val listener = object : FileReadSandbox.Listener {
+ override fun onViolation(absolutePath: String, isDirectory: Boolean) {
+ violations.add(absolutePath)
+ }
+ }
+
+ // Activate the sandbox.
+ FileReadSandbox.activate(listener)
+ try {
+ // Access "good" files, which should be allowed.
+ goodFile.readBytes()
+ subSubDirGoodFile.readBytes()
+
+ // If a file is created by metalava, then it can be read.
+ val newFile1 = File(root, "newFile1").apply { createNewFile() }
+ newFile1.readBytes()
+
+ assertWhitelistedFilesNotReported()
+ assertViolationNotReported(badFile, badDir, badDirFile, subSubDirBadFile)
+
+ // Access "bad" files.
+ badFile.readBytes()
+ badDirFile.readBytes()
+
+ assertWhitelistedFilesNotReported()
+ assertViolationReported(badFile, badDirFile)
+ assertViolationNotReported(subSubDirBadFile)
+
+ // Clear the violations, and access badDir.
+ violations.clear()
+ badDir.listFiles()
+ root.listFiles()
+
+ assertWhitelistedFilesNotReported()
+ assertViolationReported(badDir)
+ assertViolationNotReported(badFile, badDirFile, subSubDirBadFile)
+
+ // Check "implicitly allowed access" directories.
+ violations.clear()
+ subDir.listFiles() // Implicitly allowed.
+ subSubDir.listFiles() // Implicitly allowed.
+ subSubDir2.listFiles() // *Not* implicitly allowed.
+ subSubDirGoodFile.readBytes()
+ subSubDirBadFile.readBytes() // *Not* allowed.
+ root.listFiles()
+
+ assertWhitelistedFilesNotReported()
+
+ assertViolationReported(subSubDir2, subSubDirBadFile) // These are not allowed to read.
+ } finally {
+ // Deactivate the sandbox.
+ FileReadSandbox.deactivate()
+ FileReadSandbox.reset()
+ }
+ violations.clear()
+
+ // This shouldn't trigger the listener.
+ badFile.readBytes()
+
+ assertTrue(violations.isEmpty())
+ }
+}
diff --git a/src/test/java/com/android/tools/metalava/OptionsTest.kt b/src/test/java/com/android/tools/metalava/OptionsTest.kt
index 82b06f6..cf31e96 100644
--- a/src/test/java/com/android/tools/metalava/OptionsTest.kt
+++ b/src/test/java/com/android/tools/metalava/OptionsTest.kt
@@ -19,7 +19,10 @@ package com.android.tools.metalava
import com.android.tools.metalava.doclava1.Issues
import com.android.tools.metalava.model.defaultConfiguration
import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
import org.junit.Test
+import java.io.File
import java.io.PrintWriter
import java.io.StringWriter
@@ -397,6 +400,23 @@ Extracting API Levels:
Points to the current API jar, if any
+Sandboxing:
+--no-implicit-root
+ Disable implicit root directory detection. Otherwise, metalava adds in
+ source roots implied by the source files
+--strict-input-files <file>
+ Do not read files that are not explicitly specified in the command line.
+ All violations are written to the given file. Reads on directories are
+ always allowed, but metalava still trackes reads on directories that are
+ not specified in the command line, and write them to the file.
+--strict-input-files:stack <file>
+ Same as --strict-input-files but also print stacktraces.
+--strict-input-files-exempt <files or dirs>
+ Used with --strict-input-files. Explicitly allow access to files and/or
+ directories (separated by `:). Can also be @ followed by a path to a text
+ file containing paths to the full set of files and/or directories.
+
+
Environment Variables:
METALAVA_DUMP_ARGV
Set to true to have metalava emit all the arguments it was invoked with.
@@ -523,4 +543,30 @@ $FLAGS
expectedFail = "Unknown issue id: --hide ThisIssueDoesNotExist"
)
}
+
+ @Test
+ fun `Test for --strict-input-files-exempt`() {
+ val top = temporaryFolder.newFolder()
+
+ val dir = File(top, "childdir").apply { mkdirs() }
+ val grandchild1 = File(dir, "grandchiild1").apply { createNewFile() }
+ val grandchild2 = File(dir, "grandchiild2").apply { createNewFile() }
+ val file1 = File(top, "file1").apply { createNewFile() }
+ val file2 = File(top, "file2").apply { createNewFile() }
+
+ try {
+ check(
+ extraArguments = arrayOf("--strict-input-files-exempt",
+ file1.path + File.pathSeparatorChar + dir.path)
+ )
+
+ assertTrue(FileReadSandbox.isAccessAllowed(file1))
+ assertTrue(FileReadSandbox.isAccessAllowed(grandchild1))
+ assertTrue(FileReadSandbox.isAccessAllowed(grandchild2))
+
+ assertFalse(FileReadSandbox.isAccessAllowed(file2)) // Access *not* allowed
+ } finally {
+ FileReadSandbox.reset()
+ }
+ }
}