Evaluate extras and groups when determining auditable packages (#18511)

## Summary

I've made `uv audit`'s approach to handling extras and groups
(explicitly) subtractive: we don't support flags like `--dev` (since `uv
audit` audits everything by default); instead, we only support flags
like `--no-dev`, `--no-group`, etc., that remove items from the
to-be-audited set.

To accomplish that, I've abstracted the filtering into a new
`Lock::packages_for_audit` API (maybe there's a better location for
it?). Implementation wise, it does a BFS similar to the one used in `uv
tree`. I _think_ there's some room/opportunity for DRYing there but I
wanted to keep the PR small/local 🙂

See #18506.

## Test Plan

None yet.

---------

Signed-off-by: William Woodruff <william@astral.sh>
Co-authored-by: konsti <konstin@mailbox.org>

Author: woodruffw

crates/uv-cli/src/lib.rs

@@ -1197,8 +1197,12 @@ pub enum ProjectCommand {
     Format(FormatArgs),
     /// Audit the project's dependencies.
     ///
-    /// Dependencies are audited for known vulnerabilities, as well
-    /// as 'adverse' statuses such as deprecation and quarantine.
+    /// Dependencies are audited for known vulnerabilities, as well as 'adverse' statuses such as
+    /// deprecation and quarantine.
+    ///
+    /// By default, all extras and groups within the project are audited. To exclude extras
+    /// and/or groups from the audit, use the `--no-extra`, `--no-group`, and related
+    /// options.
     #[command(
         after_help = "Use `uv help audit` for more details.",
         after_long_help = ""
@@ -5138,100 +5142,45 @@ pub struct FormatArgs {
 
 #[derive(Args)]
 pub struct AuditArgs {
-    /// Include optional dependencies from the specified extra name.
-    ///
-    /// May be provided more than once.
-    ///
-    /// This option is only available when running in a project.
-    #[arg(
-        long,
-        conflicts_with = "all_extras",
-        conflicts_with = "only_group",
-        value_delimiter = ',',
-        value_parser = extra_name_with_clap_error,
-        value_hint = ValueHint::Other,
-    )]
-    pub extra: Option<Vec<ExtraName>>,
-
-    /// Include all optional dependencies.
-    ///
-    /// Optional dependencies are defined via `project.optional-dependencies` in a `pyproject.toml`.
-    ///
-    /// This option is only available when running in a project.
-    #[arg(long, conflicts_with = "extra", conflicts_with = "only_group")]
-    pub all_extras: bool,
-
-    /// Exclude the specified optional dependencies, if `--all-extras` is supplied.
+    /// Don't audit the specified optional dependencies.
     ///
     /// May be provided multiple times.
     #[arg(long, value_hint = ValueHint::Other)]
     pub no_extra: Vec<ExtraName>,
 
-    #[arg(long, overrides_with("all_extras"), hide = true)]
-    pub no_all_extras: bool,
-
-    /// Include the development dependency group [env: UV_DEV=]
-    ///
-    /// Development dependencies are defined via `dependency-groups.dev` or
-    /// `tool.uv.dev-dependencies` in a `pyproject.toml`.
-    ///
-    /// This option is an alias for `--group dev`.
-    ///
-    /// This option is only available when running in a project.
-    #[arg(long, overrides_with("no_dev"), hide = true, value_parser = clap::builder::BoolishValueParser::new())]
-    pub dev: bool,
-
-    /// Disable the development dependency group [env: UV_NO_DEV=]
+    /// Don't audit the development dependency group [env: UV_NO_DEV=]
     ///
     /// This option is an alias of `--no-group dev`.
-    /// See `--no-default-groups` to disable all default groups instead.
+    /// See `--no-default-groups` to exclude all default groups instead.
     ///
     /// This option is only available when running in a project.
-    #[arg(long, overrides_with("dev"), value_parser = clap::builder::BoolishValueParser::new())]
+    #[arg(long, value_parser = clap::builder::BoolishValueParser::new())]
     pub no_dev: bool,
 
-    /// Include dependencies from the specified dependency group.
-    ///
-    /// May be provided multiple times.
-    #[arg(long, conflicts_with_all = ["only_group", "only_dev"], value_hint = ValueHint::Other)]
-    pub group: Vec<GroupName>,
-
-    /// Disable the specified dependency group.
-    ///
-    /// This option always takes precedence over default groups,
-    /// `--all-groups`, and `--group`.
+    /// Don't audit the specified dependency group.
     ///
     /// May be provided multiple times.
     #[arg(long, env = EnvVars::UV_NO_GROUP, value_delimiter = ' ', value_hint = ValueHint::Other)]
     pub no_group: Vec<GroupName>,
 
-    /// Ignore the default dependency groups.
-    ///
-    /// uv includes the groups defined in `tool.uv.default-groups` by default.
-    /// This disables that option, however, specific groups can still be included with `--group`.
+    /// Don't audit the default dependency groups.
     #[arg(long, env = EnvVars::UV_NO_DEFAULT_GROUPS, value_parser = clap::builder::BoolishValueParser::new())]
     pub no_default_groups: bool,
 
-    /// Only include dependencies from the specified dependency group.
+    /// Only audit dependencies from the specified dependency group.
     ///
     /// The project and its dependencies will be omitted.
     ///
     /// May be provided multiple times. Implies `--no-default-groups`.
-    #[arg(long, conflicts_with_all = ["group", "dev", "all_groups"], value_hint = ValueHint::Other)]
+    #[arg(long, value_hint = ValueHint::Other)]
     pub only_group: Vec<GroupName>,
 
-    /// Include dependencies from all dependency groups.
-    ///
-    /// `--no-group` can be used to exclude specific groups.
-    #[arg(long, conflicts_with_all = ["only_group", "only_dev"])]
-    pub all_groups: bool,
-
-    /// Only include the development dependency group.
+    /// Only audit the development dependency group.
     ///
     /// The project and its dependencies will be omitted.
     ///
     /// This option is an alias for `--only-group dev`. Implies `--no-default-groups`.
-    #[arg(long, conflicts_with_all = ["group", "all_groups", "no_dev"])]
+    #[arg(long, conflicts_with_all = ["no_dev"])]
     pub only_dev: bool,
 
     /// Assert that the `uv.lock` will remain unchanged [env: UV_LOCKED=]

crates/uv-resolver/src/lock/mod.rs

@@ -15,11 +15,14 @@ use petgraph::visit::EdgeRef;
 use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
 use serde::Serializer;
 use toml_edit::{Array, ArrayOfTables, InlineTable, Item, Table, Value, value};
-use tracing::{debug, instrument};
+use tracing::{debug, instrument, trace};
 use url::Url;
 
 use uv_cache_key::RepositoryUrl;
-use uv_configuration::{BuildOptions, Constraints, InstallTarget};
+use uv_configuration::{
+    BuildOptions, Constraints, DependencyGroupsWithDefaults, ExtrasSpecificationWithDefaults,
+    InstallTarget,
+};
 use uv_distribution::{DistributionDatabase, FlatRequiresDist};
 use uv_distribution_filename::{
     BuildTag, DistExtension, ExtensionError, SourceDistExtension, WheelFilename,
@@ -810,6 +813,147 @@ impl Lock {
         )
     }
 
+    /// Returns the set of packages that should be audited, respecting the given
+    /// extras and dependency groups filters.
+    ///
+    /// Workspace members and packages without version information are excluded
+    /// unconditionally, since neither can be meaningfully looked up in a
+    /// vulnerability database.
+    pub fn packages_for_audit<'lock>(
+        &'lock self,
+        extras: &'lock ExtrasSpecificationWithDefaults,
+        groups: &'lock DependencyGroupsWithDefaults,
+    ) -> Vec<(&'lock PackageName, &'lock Version)> {
+        // Enqueue a dependency for auditability checks: base package (no extra) first, then each activated extra.
+        fn enqueue_dep<'lock>(
+            lock: &'lock Lock,
+            seen: &mut FxHashSet<(&'lock PackageId, Option<&'lock ExtraName>)>,
+            queue: &mut VecDeque<(&'lock Package, Option<&'lock ExtraName>)>,
+            dep: &'lock Dependency,
+        ) {
+            let dep_pkg = lock.find_by_id(&dep.package_id);
+            for maybe_extra in std::iter::once(None).chain(dep.extra.iter().map(Some)) {
+                if seen.insert((&dep.package_id, maybe_extra)) {
+                    queue.push_back((dep_pkg, maybe_extra));
+                }
+            }
+        }
+
+        // Identify workspace members (the implicit root counts for single-member workspaces).
+        let workspace_member_ids: FxHashSet<&PackageId> = if self.members().is_empty() {
+            self.root().into_iter().map(|package| &package.id).collect()
+        } else {
+            self.packages
+                .iter()
+                .filter(|package| self.members().contains(&package.id.name))
+                .map(|package| &package.id)
+                .collect()
+        };
+
+        // Lockfile traversal state: (package, optional extra to activate on that package).
+        let mut queue: VecDeque<(&Package, Option<&ExtraName>)> = VecDeque::new();
+        let mut seen: FxHashSet<(&PackageId, Option<&ExtraName>)> = FxHashSet::default();
+
+        // Seed from workspace members. Always queue with `None` so that we can traverse
+        // their dependency groups; only queue extras when prod mode is active.
+        for package in self
+            .packages
+            .iter()
+            .filter(|p| workspace_member_ids.contains(&p.id))
+        {
+            if seen.insert((&package.id, None)) {
+                queue.push_back((package, None));
+            }
+            if groups.prod() {
+                for extra in extras.extra_names(package.optional_dependencies.keys()) {
+                    if seen.insert((&package.id, Some(extra))) {
+                        queue.push_back((package, Some(extra)));
+                    }
+                }
+            }
+        }
+
+        // Seed from requirements attached directly to the lock (e.g., PEP 723 scripts).
+        for requirement in self.requirements() {
+            for package in self
+                .packages
+                .iter()
+                .filter(|p| p.id.name == requirement.name)
+            {
+                if seen.insert((&package.id, None)) {
+                    queue.push_back((package, None));
+                }
+            }
+        }
+
+        // Seed from dependency groups attached directly to the lock (e.g., project-less
+        // workspace roots).
+        for (group, requirements) in self.dependency_groups() {
+            if !groups.contains(group) {
+                continue;
+            }
+            for requirement in requirements {
+                for package in self
+                    .packages
+                    .iter()
+                    .filter(|p| p.id.name == requirement.name)
+                {
+                    if seen.insert((&package.id, None)) {
+                        queue.push_back((package, None));
+                    }
+                }
+            }
+        }
+
+        let mut auditable: BTreeSet<(&PackageName, &Version)> = BTreeSet::default();
+
+        while let Some((package, extra)) = queue.pop_front() {
+            let is_member = workspace_member_ids.contains(&package.id);
+
+            // Collect non-workspace packages that have version information.
+            if !is_member {
+                if let Some(version) = package.version() {
+                    auditable.insert((package.name(), version));
+                } else {
+                    trace!(
+                        "Skipping audit for `{}` because it has no version information",
+                        package.name()
+                    );
+                }
+            }
+
+            // Follow allowed dependency groups.
+            if is_member && extra.is_none() {
+                for dep in package
+                    .dependency_groups
+                    .iter()
+                    .filter(|(group, _)| groups.contains(group))
+                    .flat_map(|(_, deps)| deps)
+                {
+                    enqueue_dep(self, &mut seen, &mut queue, dep);
+                }
+            }
+
+            // Follow the regular/extra dependencies for this (package, extra) pair.
+            // For workspace members in only-group mode, skip regular dependencies.
+            let dependencies: &[Dependency] = match extra {
+                Some(extra) => package
+                    .optional_dependencies
+                    .get(extra)
+                    .map(Vec::as_slice)
+                    .unwrap_or_default(),
+                None if is_member && !groups.prod() => &[],
+                None => &package.dependencies,
+            };
+
+            for dep in dependencies {
+                enqueue_dep(self, &mut seen, &mut queue, dep);
+            }
+        }
+
+        auditable.into_iter().collect()
+    }
+
     /// Return the workspace root used to generate this lock.
     pub fn root(&self) -> Option<&Package> {
         self.packages.iter().find(|package| {

crates/uv/src/commands/project/audit.rs

@@ -85,7 +85,7 @@ pub(crate) async fn audit(
         LockTarget::Workspace(_) => DefaultExtras::default(),
         LockTarget::Script(_) => DefaultExtras::default(),
     };
-    let _extras = extras.with_defaults(default_extras);
+    let extras = extras.with_defaults(default_extras);
 
     // Determine whether we're performing a universal audit.
     let universal = python_version.is_none() && python_platform.is_none();
@@ -185,37 +185,10 @@ pub(crate) async fn audit(
         )
     });
 
-    // TODO: validate the sets of requested extras/groups against the lockfile?
-
-    // Build the list of auditable packages, skipping workspace members. Workspace members are
-    // local by definition and have no meaningful external package identity to look up in a vuln
-    // service. We also skip packages without a version, since we can't query for them.
-    //
-    // This mirrors the logic in `TreeDisplay::new`: for single-member workspaces, `lock.members()`
-    // is empty and the root package (source at path "") is the implicit member.
-    let workspace_root_name = lock.root().map(uv_resolver::Package::name);
-    let auditable: Vec<_> = lock
-        .packages()
-        .iter()
-        .filter(|p| {
-            if lock.members().is_empty() {
-                // Single-member workspace: skip the implicit root.
-                workspace_root_name != Some(p.name())
-            } else {
-                !lock.members().contains(p.name())
-            }
-        })
-        .filter_map(|p| {
-            let Some(version) = p.version() else {
-                trace!(
-                    "Skipping audit for {} because it has no version information",
-                    p.name()
-                );
-                return None;
-            };
-            Some((p.name(), version))
-        })
-        .collect();
+    // Build the list of auditable packages by traversing the lockfile from workspace roots,
+    // respecting the user's extras and dependency-group filters. Workspace members are excluded
+    // (they are local and have no external package identity), as are packages without a version.
+    let auditable = lock.packages_for_audit(&extras, &groups);
 
     // Perform the audit.
     let reporter = AuditReporter::from(printer);
@@ -247,9 +220,7 @@ pub(crate) async fn audit(
         n_packages: auditable.len(),
         findings: all_findings,
     };
-    display.render()?;
-
-    Ok(ExitStatus::Success)
+    display.render()
 }
 
 struct AuditResults {
@@ -259,7 +230,7 @@ struct AuditResults {
 }
 
 impl AuditResults {
-    fn render(&self) -> Result<()> {
+    fn render(&self) -> Result<ExitStatus> {
         let (vulns, statuses): (Vec<_>, Vec<_>) =
             self.findings.iter().partition_map(|finding| match finding {
                 Finding::Vulnerability(vuln) => itertools::Either::Left(vuln),
@@ -292,6 +263,8 @@ impl AuditResults {
             packages = format!("{npackages} packages", npackages = self.n_packages).bold()
         )?;
 
+        let has_findings = !vulns.is_empty() || !statuses.is_empty();
+
         if !vulns.is_empty() {
             writeln!(self.printer.stdout_important(), "\nVulnerabilities:\n")?;
 
@@ -357,6 +330,10 @@ impl AuditResults {
             // any adverse project statuses at the moment.
         }
 
-        Ok(())
+        if has_findings {
+            Ok(ExitStatus::Failure)
+        } else {
+            Ok(ExitStatus::Success)
+        }
     }
 }