Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 756a2f81 authored by zhidou's avatar zhidou Committed by Zhi Dou
Browse files

add filter command in exported_flag_check

This change add filter-api-flags command to this tool. It should be
triggered when the build system creates the exported flags jar of all
flags for API. It will remove all flags exported but not for APIs from
the final jar.

Test: atest exported-flag-check-test
Bug: 345798636
Change-Id: I3af0b915a6f3e6ba07d41a5f24ecac7f7f8f37a6
parent f7a35a06
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ rust_defaults {
        "libaconfig_protos",
        "libanyhow",
        "libclap",
        "libprotobuf",
        "libregex",
    ],
}
+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"
+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]);
    }
}
+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(),
            ])
        );
    }
+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: ""
}
Loading