From a3d7915c9e53125e9e26278e619eb53b24b7a08f Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Thu, 28 Oct 2021 18:14:04 -0400 Subject: [PATCH] Add comments and clarify errors in neverallow Sample of new error for violation: error: system/bt/gd/rust/topshim/macros/Android.bp:10:1: module "libtopshim_macros" variant "linux_glibc_x86_64": violates neverallow requirements. Not allowed: module types: ["rust_benchmark" "rust_benchmark_host" "rust_binary" "rust_binary_host" "rust_library" "rust_library_dylib" "rust_library_rlib" "rust_ffi" "rust_ffi_shared" "rust_ffi_static" "rust_fuzz" "rust_library_host" "rust_library_host_dylib" "rust_library_host_rlib" "rust_ffi_host" "rust_ffi_host_shared" "rust_ffi_host_static" "rust_proc_macro" "rust_test" "rust_test_host"] EXCEPT in dirs: ["device/google/cuttlefish/" "external/adhd/" "external/crosvm/" "external/libchromeos-rs/" "external/minijail/" "external/rust/" "external/selinux/libselinux/" "external/uwb/" "external/vm_tools/p9/" "frameworks/native/libs/binder/rust/" "frameworks/proto_logging/stats/" "packages/modules/DnsResolver/" "packages/modules/Virtualization/" "prebuilts/rust/" "system/core/libstats/pull_rust/" "system/extras/profcollectd/" "system/extras/simpleperf/" "system/hardware/interfaces/keystore2/" "system/librustutils/" "system/logging/liblog/" "system/logging/rust/" "system/nfc/" "system/security/" "system/tools/aidl/" "tools/security/fuzzing/example_rust_fuzzer/" "tools/security/fuzzing/orphans/" "vendor/"] Old error: error: system/bt/gd/rust/topshim/macros/Android.bp:10:1: module "libtopshim_macros" variant "linux_glibc_x86_64": neverallow -dir:device/google/cuttlefish/* -dir:external/adhd/* -dir:external/crosvm/* -dir:external/libchromeos-rs/* -dir:external/minijail/* -dir:external/rust/* -dir:external/selinux/libselinux/* -dir:external/uwb/* -dir:external/vm_tools/p9/* -dir:frameworks/native/libs/binder/rust/* -dir:frameworks/proto_logging/stats/* -dir:packages/modules/DnsResolver/* -dir:packages/modules/Virtualization/* -dir:prebuilts/rust/* -dir:system/core/libstats/pull_rust/* -dir:system/extras/profcollectd/* -dir:system/extras/simpleperf/* -dir:system/hardware/interfaces/keystore2/* -dir:system/librustutils/* -dir:system/logging/liblog/* -dir:system/logging/rust/* -dir:system/nfc/* -dir:system/security/* -dir:system/tools/aidl/* -dir:tools/security/fuzzing/example_rust_fuzzer/* -dir:tools/security/fuzzing/orphans/* -dir:vendor/* type:"rust_benchmark" type:"rust_benchmark_host type:rust_binary type:rust_binary_host type:rust_library type:rust_library_dylib type:rust_library_rlib type:rust_ffi type:rust_ffi_shared type:rust_ffi_static type:rust_fuzz type:rust_library_host type:rust_library_host_dylib type:rust_library_host_rlib type:rust_ffi_host type:rust_ffi_host_shared type:rust_ffi_host_static type:rust_proc_macro type:rust_test type:rust_test_host Test: go test soong tests Change-Id: I1a7ee6bbc8258dfffa5a76f02c12fb1e54fdba1a --- android/neverallow.go | 80 ++++++++++++++++++++++++++------------ android/neverallow_test.go | 33 +++++++++++++++- apex/apex_test.go | 4 +- 3 files changed, 89 insertions(+), 28 deletions(-) diff --git a/android/neverallow.go b/android/neverallow.go index 034861958..4bb3e57b5 100644 --- a/android/neverallow.go +++ b/android/neverallow.go @@ -15,6 +15,7 @@ package android import ( + "fmt" "path/filepath" "reflect" "regexp" @@ -372,6 +373,20 @@ type ruleProperty struct { matcher ValueMatcher } +func (r *ruleProperty) String() string { + return fmt.Sprintf("%q matches: %s", strings.Join(r.fields, "."), r.matcher) +} + +type ruleProperties []ruleProperty + +func (r ruleProperties) String() string { + var s []string + for _, r := range r { + s = append(s, r.String()) + } + return strings.Join(s, " ") +} + // A NeverAllow rule. type Rule interface { In(path ...string) Rule @@ -413,8 +428,8 @@ type rule struct { moduleTypes []string unlessModuleTypes []string - props []ruleProperty - unlessProps []ruleProperty + props ruleProperties + unlessProps ruleProperties onlyBootclasspathJar bool } @@ -424,16 +439,19 @@ func NeverAllow() Rule { return &rule{directDeps: make(map[string]bool)} } +// In adds path(s) where this rule applies. func (r *rule) In(path ...string) Rule { r.paths = append(r.paths, cleanPaths(path)...) return r } +// NotIn adds path(s) to that this rule does not apply to. func (r *rule) NotIn(path ...string) Rule { r.unlessPaths = append(r.unlessPaths, cleanPaths(path)...) return r } +// InDirectDeps adds dep(s) that are not allowed with this rule. func (r *rule) InDirectDeps(deps ...string) Rule { for _, d := range deps { r.directDeps[d] = true @@ -441,25 +459,30 @@ func (r *rule) InDirectDeps(deps ...string) Rule { return r } +// WithOsClass adds osClass(es) that this rule applies to. func (r *rule) WithOsClass(osClasses ...OsClass) Rule { r.osClasses = append(r.osClasses, osClasses...) return r } +// ModuleType adds type(s) that this rule applies to. func (r *rule) ModuleType(types ...string) Rule { r.moduleTypes = append(r.moduleTypes, types...) return r } +// NotModuleType adds type(s) that this rule does not apply to.. func (r *rule) NotModuleType(types ...string) Rule { r.unlessModuleTypes = append(r.unlessModuleTypes, types...) return r } +// With specifies property/value combinations that are restricted for this rule. func (r *rule) With(properties, value string) Rule { return r.WithMatcher(properties, selectMatcher(value)) } +// WithMatcher specifies property/matcher combinations that are restricted for this rule. func (r *rule) WithMatcher(properties string, matcher ValueMatcher) Rule { r.props = append(r.props, ruleProperty{ fields: fieldNamesForProperties(properties), @@ -468,10 +491,12 @@ func (r *rule) WithMatcher(properties string, matcher ValueMatcher) Rule { return r } +// Without specifies property/value combinations that this rule does not apply to. func (r *rule) Without(properties, value string) Rule { return r.WithoutMatcher(properties, selectMatcher(value)) } +// Without specifies property/matcher combinations that this rule does not apply to. func (r *rule) WithoutMatcher(properties string, matcher ValueMatcher) Rule { r.unlessProps = append(r.unlessProps, ruleProperty{ fields: fieldNamesForProperties(properties), @@ -487,49 +512,54 @@ func selectMatcher(expected string) ValueMatcher { return &equalMatcher{expected: expected} } +// Because specifies a reason for this rule. func (r *rule) Because(reason string) Rule { r.reason = reason return r } +// BootclasspathJar whether this rule only applies to Jars in the Bootclasspath func (r *rule) BootclasspathJar() Rule { r.onlyBootclasspathJar = true return r } func (r *rule) String() string { - s := "neverallow" - for _, v := range r.paths { - s += " dir:" + v + "*" + s := []string{"neverallow requirements. Not allowed:"} + if len(r.paths) > 0 { + s = append(s, fmt.Sprintf("in dirs: %q", r.paths)) } - for _, v := range r.unlessPaths { - s += " -dir:" + v + "*" + if len(r.moduleTypes) > 0 { + s = append(s, fmt.Sprintf("module types: %q", r.moduleTypes)) } - for _, v := range r.moduleTypes { - s += " type:" + v + if len(r.props) > 0 { + s = append(s, fmt.Sprintf("properties matching: %s", r.props)) } - for _, v := range r.unlessModuleTypes { - s += " -type:" + v + if len(r.directDeps) > 0 { + s = append(s, fmt.Sprintf("dep(s): %q", SortedStringKeys(r.directDeps))) } - for _, v := range r.props { - s += " " + strings.Join(v.fields, ".") + v.matcher.String() - } - for _, v := range r.unlessProps { - s += " -" + strings.Join(v.fields, ".") + v.matcher.String() - } - for k := range r.directDeps { - s += " deps:" + k - } - for _, v := range r.osClasses { - s += " os:" + v.String() + if len(r.osClasses) > 0 { + s = append(s, fmt.Sprintf("os class(es): %q", r.osClasses)) } if r.onlyBootclasspathJar { - s += " inBcp" + s = append(s, "in bootclasspath jar") + } + if len(r.unlessPaths) > 0 { + s = append(s, fmt.Sprintf("EXCEPT in dirs: %q", r.unlessPaths)) + } + if len(r.unlessModuleTypes) > 0 { + s = append(s, fmt.Sprintf("EXCEPT module types: %q", r.unlessModuleTypes)) + } + if len(r.unlessProps) > 0 { + s = append(s, fmt.Sprintf("EXCEPT properties matching: %q", r.unlessProps)) } if len(r.reason) != 0 { - s += " which is restricted because " + r.reason + s = append(s, " which is restricted because "+r.reason) } - return s + if len(s) == 1 { + s[0] = "neverallow requirements (empty)" + } + return strings.Join(s, "\n\t") } func (r *rule) appliesToPath(dir string) bool { diff --git a/android/neverallow_test.go b/android/neverallow_test.go index 18a870501..58a90b307 100644 --- a/android/neverallow_test.go +++ b/android/neverallow_test.go @@ -15,6 +15,7 @@ package android import ( + "regexp" "testing" "github.com/google/blueprint" @@ -55,7 +56,37 @@ var neverallowTests = []struct { }`), }, expectedErrors: []string{ - `module "libother": violates neverallow deps:not_allowed_in_direct_deps`, + regexp.QuoteMeta("module \"libother\": violates neverallow requirements. Not allowed:\n\tdep(s): [\"not_allowed_in_direct_deps\"]"), + }, + }, + { + name: "multiple constraints", + rules: []Rule{ + NeverAllow(). + InDirectDeps("not_allowed_in_direct_deps"). + In("other"). + ModuleType("cc_library"). + NotIn("top"). + NotModuleType("cc_binary"), + }, + fs: map[string][]byte{ + "top/Android.bp": []byte(` + cc_library { + name: "not_allowed_in_direct_deps", + }`), + "other/Android.bp": []byte(` + cc_library { + name: "libother", + static_libs: ["not_allowed_in_direct_deps"], + }`), + }, + expectedErrors: []string{ + regexp.QuoteMeta(`module "libother": violates neverallow requirements. Not allowed: + in dirs: ["other/"] + module types: ["cc_library"] + dep(s): ["not_allowed_in_direct_deps"] + EXCEPT in dirs: ["top/"] + EXCEPT module types: ["cc_binary"]`), }, }, diff --git a/apex/apex_test.go b/apex/apex_test.go index 9ab8ca1c5..727a1f2ec 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -7457,7 +7457,7 @@ func TestApexPermittedPackagesRules(t *testing.T) { }, { name: "Bootclasspath apex jar not satisfying allowed module packages on Q.", - expectedError: `module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar with min_sdk < T. Please jarjar or move code around.`, + expectedError: `(?s)module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar with min_sdk < T. Please jarjar or move code around.`, bp: ` java_library { name: "bcp_lib1", @@ -7494,7 +7494,7 @@ func TestApexPermittedPackagesRules(t *testing.T) { }, { name: "Bootclasspath apex jar not satisfying allowed module packages on R.", - expectedError: `module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar with min_sdk < T. Please jarjar or move code around.`, + expectedError: `(?s)module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only allow these packages: foo.bar with min_sdk < T. Please jarjar or move code around.`, bp: ` java_library { name: "bcp_lib1",