Merge "add filter command in exported_flag_check" into main (8b1c9629) · Commits · e / os / android_build

tools/aconfig/exported_flag_check/Android.bp

+1 −0

Original line number	Diff line number	Diff line
		@@ -12,6 +12,7 @@ rust_defaults {
		"libaconfig_protos",
		"libanyhow",
		"libclap",
		"libprotobuf",
		"libregex",
		],
		}

tools/aconfig/exported_flag_check/Cargo.toml

+1 −0

Original line number	Diff line number	Diff line
		@@ -11,4 +11,5 @@ cargo = []
		aconfig_protos = { path = "../aconfig_protos" }
		anyhow = "1.0.69"
		clap = { version = "4.1.8", features = ["derive"] }
		protobuf = "3.2.0"
		regex = "1.11.1"

tools/aconfig/exported_flag_check/src/main.rs

+118 −45

Original line number	Diff line number	Diff line
		@@ -15,18 +15,19 @@
		*/

		//! `exported-flag-check` is a tool to ensures that exported flags are used as intended
		use anyhow::{ensure, Result};
		use clap::Parser;
		use std::{collections::HashSet, fs::File, path::PathBuf};
		use anyhow::{anyhow, bail, ensure, Context, Result};
		use clap::{builder::ArgAction, Arg, ArgMatches, Command};
		use std::io::Write;
		use std::{collections::HashSet, fs, fs::File, io::Read, path::PathBuf};

		mod utils;

		use utils::{
		check_all_exported_flags, extract_flagged_api_flags, get_exported_flags_from_binary_proto,
		read_finalized_flags,
		check_all_exported_flags, extract_flagged_api_flags, filter_api_flags,
		get_exported_flags_from_binary_proto, read_flag_from_binary, FlagId,
		};

		const ABOUT: &str = "CCheck Exported Flags
		const HELP: &str = "CCheck Exported Flags

		This tool ensures that exported flags are used as intended. Exported flags, marked with
		`is_exported: true` in their declaration, are designed to control access to specific API
		@@ -44,49 +45,120 @@ This tool works as follows:
		- Error if exported flags are not in the set
		";

		#[derive(Parser, Debug)]
		#[clap(about=ABOUT)]
		struct Cli {
		#[arg(long)]
		parsed_flags_file: PathBuf,

		#[arg(long)]
		api_signature_file: Vec<PathBuf>,
		fn cli() -> Command {
		Command::new("exported-flag-check")
		.subcommand_required(true)
		.subcommand(
		Command::new("validate-exported-flags")
		.arg(Arg::new("parsed-flags-file").long("parsed-flags-file").required(true))
		.arg(
		Arg::new("api-signature-file")
		.long("api-signature-file")
		.required(true)
		.action(ArgAction::Append),
		)
		.arg(Arg::new("finalized-flags-file").long("finalized-flags-file").required(true)),
		)
		.subcommand(
		Command::new("filter-api-flags")
		.arg(Arg::new("cache").long("cache").required(true))
		.arg(Arg::new("out").long("out").required(true)),
		)
		.after_help(HELP.trim())
		}

		#[arg(long)]
		finalized_flags_file: PathBuf,
		fn open_single_file(matches: &ArgMatches, arg_name: &str) -> Result<Box<dyn Read>> {
		let Some(path) = matches.get_one::<String>(arg_name) else {
		bail!("missing argument {}", arg_name);
		};
		Ok(Box::new(File::open(path)?))
		}

		fn main() -> Result<()> {
		let args = Cli::parse();
		fn open_multiple_files(matches: &ArgMatches, arg_name: &str) -> Result<Vec<Box<dyn Read>>> {
		let mut opened_files: Vec<Box<dyn Read>> = Vec::new();
		for path in matches.get_many::<String>(arg_name).unwrap_or_default() {
		opened_files.push(Box::new(File::open(path)?));
		}
		Ok(opened_files)
		}

		fn validate_exported_flags<R: Read>(
		parsed_flags_file: R,
		api_signature_files: Vec<R>,
		finalized_flags_file: R,
		allow_flag_file: R,
		allow_flag_package: R,
		) -> Result<Vec<FlagId>> {
		let mut flags_used_with_flaggedapi_annotation = HashSet::new();
		for path in &args.api_signature_file {
		let file = File::open(path)?;
		for file in api_signature_files {
		let flags = extract_flagged_api_flags(file)?;
		flags_used_with_flaggedapi_annotation.extend(flags);
		}

		let file = File::open(args.parsed_flags_file)?;
		let all_flags = get_exported_flags_from_binary_proto(file)?;

		let file = File::open(args.finalized_flags_file)?;
		let already_finalized_flags = read_finalized_flags(file)?;
		let all_flags = get_exported_flags_from_binary_proto(parsed_flags_file)?;
		let already_finalized_flags = read_flag_from_binary(finalized_flags_file)?;
		let allow_flag_set = read_flag_from_binary(allow_flag_file)?;
		let allow_package_set = read_flag_from_binary(allow_flag_package)?;

		let exported_flags = check_all_exported_flags(
		&flags_used_with_flaggedapi_annotation,
		&all_flags,
		&already_finalized_flags,
		&allow_flag_set,
		&allow_package_set,
		)?;

		println!("{}", exported_flags.join("\n"));

		Ok(exported_flags)
		}

		fn main() -> Result<()> {
		let matches = cli().get_matches();
		match matches.subcommand() {
		Some(("validate-exported-flags", sub_matches)) => {
		let parsed_flags_file = open_single_file(sub_matches, "parsed-flags-file")?;
		let api_signature_files = open_multiple_files(sub_matches, "api-signature-file")?;
		let finalized_flags_file = open_single_file(sub_matches, "finalized-flags-file")?;
		let allow_flag_file = include_str!("../allow_flag_list.txt");
		let allow_flag_package = include_str!("../allow_package_list.txt");

		let exported_flags = validate_exported_flags(
		parsed_flags_file,
		api_signature_files,
		finalized_flags_file,
		Box::new(allow_flag_file.as_bytes()),
		Box::new(allow_flag_package.as_bytes()),
		)?;

		ensure!(
		exported_flags.is_empty(),
		"Flags {} are exported but not used to guard any API. \
		Exported flag should be used to guard API",
		exported_flags.join(",")
		);
		}
		Some(("filter-api-flags", sub_matches)) => {
		let cache = open_single_file(sub_matches, "cache")?;

		let Some(out_file_arg) = sub_matches.get_one::<String>("out") else {
		bail!("argument out is missing");
		};
		let out_file = PathBuf::from(out_file_arg);
		let allow_flag_file = &include_str!("../allow_flag_list.txt");
		let filtered_cache = filter_api_flags(cache, Box::new(allow_flag_file.as_bytes()))?;
		let parent = out_file
		.parent()
		.ok_or(anyhow!("unable to locate parent of output file {}", out_file.display()))?;
		fs::create_dir_all(parent)
		.with_context(\|\| format!("failed to create directory {}", parent.display()))?;
		let mut file = fs::File::create(&out_file)
		.with_context(\|\| format!("failed to open {}", out_file.display()))?;
		file.write_all(&filtered_cache)
		.with_context(\|\| format!("failed to write to {}", out_file.display()))?;
		}
		_ => unreachable!(),
		}

		Ok(())
		}

		@@ -96,22 +168,23 @@ mod tests {

		#[test]
		fn test() {
		let input = include_bytes!("../tests/api-signature-file.txt");
		let flags_used_with_flaggedapi_annotation = extract_flagged_api_flags(&input[..]).unwrap();

		let input = include_bytes!("../tests/flags.protobuf");
		let all_flags_to_be_finalized = get_exported_flags_from_binary_proto(&input[..]).unwrap();

		let input = include_bytes!("../tests/finalized-flags.txt");
		let already_finalized_flags = read_finalized_flags(&input[..]).unwrap();

		let exported_flags = check_all_exported_flags(
		&flags_used_with_flaggedapi_annotation,
		&all_flags_to_be_finalized,
		&already_finalized_flags,
		let flags_used_with_flaggedapi_annotation =
		vec![&include_bytes!("../tests/api-signature-file.txt")[..]];
		let all_flags_to_be_finalized = include_bytes!("../tests/flags.protobuf");
		let already_finalized_flags = include_bytes!("../tests/finalized-flags.txt");
		let allow_flag_file = "record_finalized_flags.test.boo".as_bytes();
		let allow_flag_package = "".as_bytes();

		let exported_flags = validate_exported_flags(
		&all_flags_to_be_finalized[..],
		flags_used_with_flaggedapi_annotation,
		&already_finalized_flags[..],
		allow_flag_file,
		allow_flag_package,
		)
		.unwrap();

		assert_eq!(1, exported_flags.len());
		assert_eq!("record_finalized_flags.test.not_enabled", exported_flags[0]);
		}
		}

tools/aconfig/exported_flag_check/src/utils.rs

+66 −28

Original line number	Diff line number	Diff line
		@@ -14,8 +14,9 @@
		* limitations under the License.
		*/

		use aconfig_protos::ParsedFlagExt;
		use anyhow::{anyhow, Context, Result};
		use aconfig_protos::{ParsedFlagExt, ProtoParsedFlags};
		use anyhow::{anyhow, Result};
		use protobuf::Message;
		use regex::Regex;
		use std::{
		collections::HashSet,
		@@ -36,11 +37,12 @@ pub(crate) fn extract_flagged_api_flags<R: Read>(mut reader: R) -> Result<HashSe

		/// Read a list of flag names. The input is expected to be plain text, with each line containing
		/// the name of a single flag.
		pub(crate) fn read_finalized_flags<R: Read>(reader: R) -> Result<HashSet<FlagId>> {
		BufReader::new(reader)
		pub(crate) fn read_flag_from_binary<R: Read>(reader: R) -> Result<HashSet<FlagId>> {
		Ok(BufReader::new(reader)
		.lines()
		.map(\|line_result\| line_result.context("Failed to read line from finalized flags file"))
		.collect()
		.map_while(Result::ok) // Ignore lines that fail to read
		.map(\|line\| line.trim().to_string())
		.collect())
		}

		/// Parse a ProtoParsedFlags binary protobuf blob and return the fully qualified names of flags
		@@ -60,28 +62,15 @@ pub(crate) fn get_exported_flags_from_binary_proto<R: Read>(
		Ok(HashSet::from_iter(iter))
		}

		fn get_allow_flag_list() -> Result<HashSet<FlagId>> {
		let allow_list: HashSet<FlagId> =
		include_str!("../allow_flag_list.txt").lines().map(\|x\| x.into()).collect();
		Ok(allow_list)
		}

		fn get_allow_package_list() -> Result<HashSet<FlagId>> {
		let allow_list: HashSet<FlagId> =
		include_str!("../allow_package_list.txt").lines().map(\|x\| x.into()).collect();
		Ok(allow_list)
		}

		/// Filter out the flags have is_exported as true but not used with @FlaggedApi annotations
		/// in the source tree, or in the previously finalized flags set.
		pub(crate) fn check_all_exported_flags(
		flags_used_with_flaggedapi_annotation: &HashSet<FlagId>,
		all_flags: &HashSet<FlagId>,
		already_finalized_flags: &HashSet<FlagId>,
		allow_flag_set: &HashSet<FlagId>,
		allow_package_set: &HashSet<FlagId>,
		) -> Result<Vec<FlagId>> {
		let allow_flag_list = get_allow_flag_list()?;
		let allow_package_list = get_allow_package_list()?;

		let new_flags: Vec<FlagId> = all_flags
		.difference(flags_used_with_flaggedapi_annotation)
		.cloned()
		@@ -89,11 +78,11 @@ pub(crate) fn check_all_exported_flags(
		.difference(already_finalized_flags)
		.cloned()
		.collect::<HashSet<_>>()
		.difference(&allow_flag_list)
		.difference(allow_flag_set)
		.filter(\|flag\| {
		if let Some(last_dot_index) = flag.rfind('.') {
		let package_name = &flag[..last_dot_index];
		!allow_package_list.contains(package_name)
		!allow_package_set.contains(package_name)
		} else {
		true
		}
		@@ -104,14 +93,34 @@ pub(crate) fn check_all_exported_flags(
		Ok(new_flags)
		}

		pub(crate) fn filter_api_flags<R: Read>(mut cache: R, non_api_flag_file: R) -> Result<Vec<u8>> {
		let non_api_flag_set = read_flag_from_binary(non_api_flag_file)?;
		let mut buffer = Vec::new();
		cache.read_to_end(&mut buffer)?;
		let parsed_flags = aconfig_protos::parsed_flags::try_from_binary_proto(&buffer)
		.map_err(\|_\| anyhow!("failed to parse binary proto"))?;
		let mut filtered_parsed_flags = ProtoParsedFlags::new();
		parsed_flags
		.parsed_flag
		.into_iter()
		.filter(\|flag\| {
		flag.is_exported() && !non_api_flag_set.contains(&flag.fully_qualified_name())
		})
		.for_each(\|flag\| filtered_parsed_flags.parsed_flag.push(flag.clone()));
		aconfig_protos::parsed_flags::sort_parsed_flags(&mut filtered_parsed_flags);
		let mut output = Vec::new();
		filtered_parsed_flags.write_to_vec(&mut output)?;
		Ok(output)
		}

		#[cfg(test)]
		mod tests {
		use super::*;

		#[test]
		fn test_extract_flagged_api_flags() {
		let api_signature_file = include_bytes!("../tests/api-signature-file.txt");
		let flags = extract_flagged_api_flags(&api_signature_file[..]).unwrap();
		let api_signature_files = include_bytes!("../tests/api-signature-file.txt");
		let flags = extract_flagged_api_flags(&api_signature_files[..]).unwrap();
		assert_eq!(
		flags,
		HashSet::from_iter(vec![
		@@ -124,7 +133,7 @@ mod tests {
		#[test]
		fn test_read_finalized_flags() {
		let input = include_bytes!("../tests/finalized-flags.txt");
		let flags = read_finalized_flags(&input[..]).unwrap();
		let flags = read_flag_from_binary(&input[..]).unwrap();
		assert_eq!(
		flags,
		HashSet::from_iter(vec![
		@@ -135,14 +144,43 @@ mod tests {
		}

		#[test]
		fn test_disabled_or_read_write_flags_are_ignored() {
		fn test_get_exported_flags_from_binary_proto() {
		let bytes = include_bytes!("../tests/flags.protobuf");
		let flags = get_exported_flags_from_binary_proto(&bytes[..]).unwrap();
		assert_eq!(
		flags,
		HashSet::from_iter(vec![
		"record_finalized_flags.test.foo".to_string(),
		"record_finalized_flags.test.not_enabled".to_string()
		"record_finalized_flags.test.not_enabled".to_string(),
		"record_finalized_flags.test.bar".to_string(),
		"record_finalized_flags.test.boo".to_string(),
		])
		);
		}

		#[test]
		fn test_filter_api_flags() {
		let bytes = include_bytes!("../tests/flags.protobuf");
		let allow_flag_file = r#"
		record_finalized_flags.test.boo
		record_finalized_flags.test.not_enabled
		"#
		.as_bytes();
		let flags = filter_api_flags(&bytes[..], allow_flag_file).unwrap();
		let parsed_flags = aconfig_protos::parsed_flags::try_from_binary_proto(&flags).unwrap();
		assert_eq!(2, parsed_flags.parsed_flag.len());

		let ret = parsed_flags
		.parsed_flag
		.into_iter()
		.filter(\|flag\| flag.is_exported())
		.map(\|flag\| flag.fully_qualified_name())
		.collect::<HashSet<FlagId>>();
		assert_eq!(
		ret,
		HashSet::from_iter(vec![
		"record_finalized_flags.test.foo".to_string(),
		"record_finalized_flags.test.bar".to_string(),
		])
		);
		}

tools/aconfig/exported_flag_check/tests/flags.declarations

+25 −2

Original line number	Diff line number	Diff line
		@@ -4,7 +4,7 @@ container: "system"
		flag {
		name: "foo"
		namespace: "test"
		description: "FIXME"
		description: "This is allow_list flag for API in the signature file"
		bug: ""
		is_exported:true
		}
		@@ -12,7 +12,30 @@ flag {
		flag {
		name: "not_enabled"
		namespace: "test"
		description: "FIXME"
		description: "This is a flag exported, but not for API"
		bug: ""
		is_exported:true
		}

		flag {
		name: "bar"
		namespace: "test"
		description: "This is a finalized flag for API"
		bug: ""
		is_exported:true
		}

		flag {
		name: "boo"
		namespace: "test"
		description: "This is a flag not for API, but allowed"
		bug: ""
		is_exported:true
		}

		flag {
		name: "far"
		namespace: "test"
		description: "This is a flag is not exported"
		bug: ""
		}