Skip to content

Groups and selectors: raw cursors and npm audits name nothing hidden #700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
schmitch merged 1 commit into from
Oct 30, 2026
Merged

Groups and selectors: raw cursors and npm audits name nothing hidden #700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions crates/plugin-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,10 @@ pub trait FormatPlugin: Send + Sync + 'static {
/// load, or an action of the host's.
fn render(&self, repo: &Repository, req: &Request, data: &[Loaded]) -> Action;

/// The body of an [`Action::Forward`] through a group without the
/// packages `hidden` names: the reserved prefixes of every group on the
/// way to the proxy keep their packages' names from the upstream.
/// The body of an [`Action::Forward`] without the packages `hidden`
/// names: the reserved prefixes of every group on the way to the proxy,
/// and the caller's content selectors on every repository on the way,
/// keep their packages' names from the upstream.
/// `hidden` takes a package id as the body writes it. `None` when the
/// body cannot be filtered: the host then refuses the request (400) and
/// never sends it on unfiltered. The default filters nothing, so a
Expand Down
156 changes: 105 additions & 51 deletions crates/server/src/files/listing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
//! whose first [`SUBDIRECTORY_SCAN`] files is visible is left out). A
//! group lists the merged entries of the members the caller may read, the
//! highest-priority member's file first; a proxy lists what it has cached.
//!
//! Nothing a selector hides is named, not even by `next`: a page reads on
//! past hidden entries until it holds `limit` visible ones, and `next` is
//! the last of them. That walk is bounded ([`LISTING_SCAN`] entries): a
//! page that reaches the bound ends at its last visible entry, and the
//! listing ends where it found none. A directory exists for the caller
//! when it holds a visible entry, so one of hidden entries alone is not
//! found, whatever its size.

use std::collections::BTreeMap;

Expand All @@ -33,6 +41,14 @@ pub const MAX_LIMIT: usize = 1000;
/// a selector-restricted caller may see.
pub const SUBDIRECTORY_SCAN: usize = 1000;

/// How many entries of a directory a page walks, at most, to find the
/// `limit` a selector-restricted caller may see (a page of `MAX_LIMIT`
/// walks one more, to know whether another follows).
pub const LISTING_SCAN: usize = 1000;

/// How many entries a selector-restricted page walks at once, at least.
const WALK_BATCH: usize = 100;

/// One entry of a page.
#[derive(Debug, Clone)]
pub struct Entry {
Expand Down Expand Up @@ -64,29 +80,33 @@ fn resume_at(prefix: &str, after: Option<&str>) -> Result<String, FileError> {
let Some(after) = after.filter(|a| !a.is_empty()) else {
return Ok(prefix.to_string());
};
let (name, directory) = match after.strip_suffix('/') {
Some(name) => (name, true),
None => (after, false),
};
let name = after.strip_suffix('/').unwrap_or(after);
files::validate_segment(name).map_err(|e| FileError::BadRequest(format!("after: {e}")))?;
Ok(if directory {
format!("{prefix}{name}0")
} else {
format!("{prefix}{name}\u{1}")
})
Ok(past(prefix, after))
}

/// The least path past the entry `entry` of directory `prefix`, a name
/// the walk read (see [`resume_at`]).
fn past(prefix: &str, entry: &str) -> String {
match entry.strip_suffix('/') {
Some(name) => format!("{prefix}{name}0"),
None => format!("{prefix}{entry}\u{1}"),
}
}

/// One repository's entries of a page: what it has after `from`, as far as
/// it read (`more`: it has entries past the last one returned, up to and
/// including the entry named `more`, which it has read).
/// it read (`more`: it may have entries past the last one returned, and
/// has all of them up to and including the entry named `more`, which it
/// returned: a name the caller may see).
struct Partial {
entries: Vec<Entry>,
more: Option<String>,
}

impl Files<'_> {
/// The entries of `prefix` in hosted or proxy `repo` from `from` on,
/// visible through every one of `filters`.
/// visible through every one of `filters`: `limit` of them, walking
/// past the hidden ones up to [`LISTING_SCAN`] entries.
async fn list_local(
&self,
repo: &Repository,
Expand All @@ -95,48 +115,82 @@ impl Files<'_> {
limit: usize,
filters: &[PackageFilter],
) -> Result<Partial, FileError> {
let walked = raw_dao::walk_directory(
&self.state.pool,
repo.id,
prefix,
from,
i64::try_from(limit + 1).unwrap_or(i64::MAX),
)
.await?;
let more = walked.len() > limit;
// One visible entry past the page says that another page follows.
let wanted = limit + 1;
let unfiltered = filters.iter().all(|f| matches!(f, PackageFilter::AllowAll));
let bound = if unfiltered {
wanted
} else {
wanted.max(LISTING_SCAN)
};
let mut entries = Vec::new();
let mut last = None;
for walked in walked.into_iter().take(limit) {
let rest = &walked.path[prefix.len()..];
let (entry, key) = match (rest.find('/'), &walked.file) {
(None, Some(file)) => {
let visible = filters.iter().all(|f| f.allows(&file.path));
let entry = Entry {
name: rest.to_string(),
kind: EntryKind::File(Box::new(file.clone())),
};
(visible.then_some(entry), rest.to_string())
}
(Some(i), _) => {
let name = &rest[..i];
let dir = format!("{prefix}{name}/");
let visible = self.directory_visible(repo, &dir, filters).await?;
let entry = Entry {
name: format!("{name}/"),
kind: EntryKind::Directory(dir),
};
(visible.then_some(entry), format!("{name}/"))
let mut walked_count = 0;
let mut from = from.to_string();
let mut ended = false;
while entries.len() < wanted && walked_count < bound {
let batch = if unfiltered {
wanted - entries.len()
} else {
(wanted - entries.len()).max(WALK_BATCH)
}
.min(bound - walked_count);
let walked = raw_dao::walk_directory(
&self.state.pool,
repo.id,
prefix,
&from,
i64::try_from(batch).unwrap_or(i64::MAX),
)
.await?;
ended = walked.len() < batch;
walked_count += walked.len();
let mut last = None;
for walked in walked {
if entries.len() == wanted {
break;
}
// A file row vanished between the walk and the join.
(None, None) => (None, rest.to_string()),
};
entries.extend(entry);
last = Some(key);
let rest = &walked.path[prefix.len()..];
let (entry, key) = match (rest.find('/'), &walked.file) {
(None, Some(file)) => {
let visible = filters.iter().all(|f| f.allows(&file.path));
let entry = Entry {
name: rest.to_string(),
kind: EntryKind::File(Box::new(file.clone())),
};
(visible.then_some(entry), rest.to_string())
}
(Some(i), _) => {
let name = &rest[..i];
let dir = format!("{prefix}{name}/");
let visible = self.directory_visible(repo, &dir, filters).await?;
let entry = Entry {
name: format!("{name}/"),
kind: EntryKind::Directory(dir),
};
(visible.then_some(entry), format!("{name}/"))
}
// A file row vanished between the walk and the join.
(None, None) => (None, rest.to_string()),
};
entries.extend(entry);
last = Some(key);
}
match last {
Some(last) if !ended => from = past(prefix, &last),
_ => break,
}
}
Ok(Partial {
entries,
more: if more { last } else { None },
})
// Past the page, at the walk's bound or at the directory's end, the
// page ends at its last visible entry: never at a hidden one.
let more = if entries.len() > limit {
entries.truncate(limit);
entries.last().map(|e| e.name.clone())
} else if ended {
None
} else {
entries.last().map(|e| e.name.clone())
};
Ok(Partial { entries, more })
}

/// Whether directory `dir` of `repo` holds a file visible through every
Expand Down
89 changes: 61 additions & 28 deletions crates/server/src/plugins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use axum::response::{IntoResponse, Response};
use bytes::Bytes;
use forge_core::auth::principal::{AuthMethod, Principal};
use forge_core::auth::token::{check_token, TokenCheck};
use forge_core::authz::Level;
use forge_core::authz::{Level, PackageFilter};
use forge_core::format::Format;
use forge_core::net::upstream::{RequestClass, SpooledBody};
use forge_core::repo::{
Expand Down Expand Up @@ -1627,17 +1627,24 @@ impl Ctx<'_> {

/// The proxy whose upstream a forward from `repo` goes to: the
/// repository itself, or a group's first proxy member (by priority)
/// the caller may read, through nested groups. With it, the reserved
/// prefixes of every group on the way, whose packages the forward
/// leaves out: reading through an outer group keeps an inner group's
/// packages inside as reading it directly does.
/// the caller may read, through nested groups. With it, what the
/// forward leaves out ([`Withheld`]): the reserved prefixes of every
/// group on the way, so that reading through an outer group keeps an
/// inner group's packages inside as reading it directly does, and the
/// packages the caller's selectors hide on any repository on the way.
async fn forward_source(
&self,
repo: &Repository,
depth: u32,
) -> Result<Option<(Repository, Vec<String>)>, RepoError> {
) -> Result<Option<(Repository, Withheld)>, RepoError> {
match repo.kind() {
RepoKind::Proxy => Ok(Some((repo.clone(), Vec::new()))),
RepoKind::Proxy => Ok(Some((
repo.clone(),
Withheld {
reserved: Vec::new(),
filters: vec![self.state.authz.package_filter(&self.principal, repo).await],
},
))),
RepoKind::Hosted => Ok(None),
RepoKind::Group => {
let max = self.state.config.groups.max_nesting_depth.max(1) as u32;
Expand All @@ -1648,11 +1655,14 @@ impl Ctx<'_> {
if member.format != repo.format || !self.may(Level::Read, &member).await {
continue;
}
if let Some((proxy, mut reserved)) =
if let Some((proxy, mut withheld)) =
Box::pin(self.forward_source(&member, depth + 1)).await?
{
reserved.extend(repo.reserved_prefixes());
return Ok(Some((proxy, reserved)));
withheld.reserved.extend(repo.reserved_prefixes());
withheld
.filters
.push(self.state.authz.package_filter(&self.principal, repo).await);
return Ok(Some((proxy, withheld)));
}
}
Ok(None)
Expand All @@ -1661,11 +1671,12 @@ impl Ctx<'_> {
}

/// Send `forward` on to the upstream, with the proxy's credentials (to
/// its origin only), and answer with what the upstream said. Through a
/// group with reserved prefixes on the way, the body goes without their
/// packages ([`FormatPlugin::forward_body`]), or not at all.
/// its origin only), and answer with what the upstream said. With
/// reserved prefixes or the caller's selectors on the way, the body goes
/// without the packages they keep inside ([`FormatPlugin::forward_body`]),
/// or not at all.
async fn forward(&self, forward: Forward) -> Response {
let (source, reserved) = match self.forward_source(self.repo, 0).await {
let (source, withheld) = match self.forward_source(self.repo, 0).await {
Ok(Some(found)) => found,
Ok(None) => {
return (
Expand All @@ -1691,29 +1702,19 @@ impl Ctx<'_> {
};
let url = format!("{}{}", remote.trim_end_matches('/'), forward.path);
let content_type = forward.content_type.clone();
let body = if reserved.is_empty() {
let body = if withheld.is_empty() {
forward.body
} else {
let filtered = self
.call(move |plugin| {
// Prefixes are stored lowercase; a name is matched in
// any case, which keeps more inside, never less.
let hidden = |name: &str| {
let lower = name.to_lowercase();
reserved
.iter()
.any(|prefix| lower.starts_with(prefix.as_str()))
};
plugin.forward_body(&forward, &hidden)
})
.call(move |plugin| plugin.forward_body(&forward, &|name| withheld.hides(name)))
.await;
match filtered {
Ok(Some(body)) => body,
Ok(None) => {
return (
StatusCode::BAD_REQUEST,
"this request cannot be passed on without the packages this \
repository keeps from its upstream (its reserved prefixes)",
"this request cannot be passed on without the packages it must \
leave out: those under reserved prefixes, and those you may not read",
)
.into_response()
}
Expand Down Expand Up @@ -1868,6 +1869,38 @@ impl Ctx<'_> {
}
}

/// What a forward leaves out of its body on the way to a proxy's upstream
/// ([`Ctx::forward_source`]).
struct Withheld {
/// The reserved prefixes of the groups on the way, lowercase.
reserved: Vec<String>,
/// The caller's package filter on each repository on the way, the
/// proxy's included.
filters: Vec<PackageFilter>,
}

impl Withheld {
/// Nothing is left out: the body goes on as it is.
fn is_empty(&self) -> bool {
self.reserved.is_empty()
&& self
.filters
.iter()
.all(|f| matches!(f, PackageFilter::AllowAll))
}

/// Whether the package `name` (as the body writes it) stays inside.
/// Prefixes are stored lowercase, and selectors match a name in any
/// case: a name keeps more inside, never less.
fn hides(&self, name: &str) -> bool {
let lower = name.to_lowercase();
self.reserved
.iter()
.any(|prefix| lower.starts_with(prefix.as_str()))
|| !self.filters.iter().all(|f| f.allows(name))
}
}

/// The permission `change` needs, and what it is called in a refusal.
fn needs(change: &Change) -> (Level, &'static str) {
match change {
Expand Down
Loading
Loading