Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 24 additions & 11 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,13 @@ jobs:
GH_TOKEN: ${{ github.token }}
run: .github/scripts/ci-changes.sh

# ── Formatting, the changelog fragments, the docs' links, xtask's tests ────
# The fragment and link checks and the tests build only xtask, which has no
# dependencies. On a pull request the fragment check sees the merge with
# the base, so a fragment under a milestone that was released after the
# pull request was written fails. xtask's tests run in `test` as well; here
# they also run for a change to the files they read outside xtask (the
# release group), which leaves the rest of the Rust jobs out.
# ── Formatting, changelog fragments, docs' links, unused code, xtask tests ──
# The fragment, link and unused-code checks and the tests build only xtask,
# which has no dependencies. On a pull request the fragment check sees the
# merge with the base, so a fragment under a milestone that was released
# after the pull request was written fails. xtask's tests run in `test` as
# well; here they also run for a change to the files they read outside
# xtask (the release group), which leaves the rest of the Rust jobs out.
# No job waits for this one (`needs: [fmt]`): a job behind `needs` joins
# the runner queue only once fmt is done, so when the runners are busy the
# long jobs would queue twice (#337).
Expand All @@ -158,13 +158,18 @@ jobs:
# to a renamed file).
- name: Documentation links (docs/SUMMARY.md)
run: cargo xtask docs --check
# What rustc and clippy cannot see is unused: a pub item nothing else
# names, a dependency its crate never imports, a dev-dependency that
# repeats a dependency, a workspace dependency no member inherits.
- name: Unused code and dependencies (cargo xtask unused)
run: cargo xtask unused
- name: cargo test -p xtask
run: cargo test -p xtask

# ── Clippy, and the sqlx offline-cache drift guard ─────────────────────────
# Both are check builds; the drift guard runs here rather than in `test`,
# whose build it shares nothing with (--all-features), to keep it off the
# longest job's path.
# ── Clippy, the sqlx offline-cache drift guard, the API docs' links ────────
# All are check builds; the drift guard and the docs run here rather than in
# `test`, whose build they share nothing with (--all-features), to keep them
# off the longest job's path.
clippy:
name: clippy
runs-on: ubuntu-latest
Expand Down Expand Up @@ -220,6 +225,11 @@ jobs:
env:
SQLX_OFFLINE: ""
run: cargo sqlx prepare --check --workspace -- --all-targets --all-features
# rustdoc with its warnings denied: a doc link to a renamed or removed
# item fails. After the drift guard, whose check build of every feature
# leaves only the crates' docs to build.
- name: API docs (cargo xtask doc-check)
run: cargo xtask doc-check

# ── Dependency advisories (RustSec) ────────────────────────────────────────
# Fails on any known vulnerability in Cargo.lock. Accepted advisories, each
Expand Down Expand Up @@ -446,6 +456,9 @@ jobs:
run: pnpm install --frozen-lockfile
- name: Typecheck (tsc --noEmit)
run: pnpm run tsc
# Files, exports and dependencies nothing uses (web/knip.jsonc).
- name: Unused code (knip)
run: pnpm run knip
- name: Unit tests (Vitest)
run: pnpm run test
- name: Production build (vite build -> web/dist)
Expand Down
43 changes: 42 additions & 1 deletion DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ URLs before handing off to Vite.
cargo xtask web-test # Vitest + React Testing Library (unit/component)
cargo xtask web-e2e # Playwright e2e (needs Docker; builds the UI first)
cd web && pnpm run tsc # typecheck
cd web && pnpm run knip # files, exports and dependencies nothing uses
cd web && pnpm run e2e:install # one-time: download the Playwright browser
```

Expand Down Expand Up @@ -793,6 +794,45 @@ dependencies that are slow without it, `rsa` and `argon2` (`Cargo.toml`,
`[profile.dev.package.*]`): the mock IdPs generate RSA keys, and every test
server hashes its bootstrap admin password.

## Unused code and stale doc links

rustc warns about a private function nothing calls, but not about a `pub`
one, which another crate might call; neither Cargo nor `tsc` says when a
dependency is no longer imported; and a doc comment that links an item
renamed or removed since still compiles. Three checks catch what they leave,
and CI runs all three:

- **`cargo xtask unused`** (the `fmt` job) fails on a `pub` item (a function,
type, trait, constant or static) whose name appears nowhere else in the
workspace's Rust sources; a dependency its crate's sources never name
(`name::…`, `use name`); a dev-dependency that repeats one of its crate's
dependencies without adding a feature; and an entry of
`[workspace.dependencies]` that no member inherits. It searches the sources
as text and errs towards silence: a name that also appears elsewhere (in a
comment, a string, as a local) counts as used. Delete what it reports. A dependency the code needs without naming it (one
that only turns on a feature of another) goes, with the reason, into its
crate's `Cargo.toml`:

```toml
[package.metadata.unused]
# <why the code needs it without naming it>
ignore = ["<name>"]
```

- **`cd web && pnpm run knip`** (the `web` job) fails on a file of `web/`
nothing imports, an export no other module imports, and a package of
`package.json` nothing imports ([knip](https://knip.dev), configured in
`web/knip.jsonc`). An export only its own module uses is left alone.

- **`cargo xtask doc-check`** (the `clippy` job) builds the crates' API docs
(`cargo doc --workspace --all-features --no-deps --document-private-items`)
with rustdoc's warnings denied, so a doc link (`` [`name`] ``) to an item
that is gone fails, as does an ambiguous one. Links from public to private
items are allowed, since the private items are documented too.

None of them knows whether a comment's prose is still true: that stays with
the review.

## Resetting & troubleshooting

- `cargo xtask down` stops the containers but **keeps** the Postgres volume. To
Expand All @@ -819,7 +859,8 @@ server hashes its bootstrap admin password.

`up` · `down` · `dev` · `run` · `seed` · `test` · `e2e` · `check` · `fmt` ·
`clippy` · `build` · `migrate` · `prepare` (regenerate sqlx offline data) ·
`prepare-check` · `psql` · `logs` · `ps` · `clean`.
`prepare-check` · `unused` · `doc-check` (both: see [Unused code and stale doc
links](#unused-code-and-stale-doc-links)) · `psql` · `logs` · `ps` · `clean`.

Web UI: `web` (build) · `web-install` · `web-dev` (Vite HMR) · `web-test`
(Vitest) · `web-e2e` (Playwright).
Expand Down
2 changes: 1 addition & 1 deletion crates/cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
//! ## TTL approach
//!
//! moka's own per-entry TTL (via an `Expiry`) governs L1. We *also* stamp each
//! stored entry with an absolute `expires_at` [`Instant`] and treat an entry
//! stored entry with an absolute `expires_at` [`Instant`](std::time::Instant) and treat an entry
//! whose deadline has passed as a **miss** on read. Belt-and-suspenders: it
//! makes expiry deterministic for tests (which can't depend on moka's
//! background eviction timing) while moka's expiry bounds memory.
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/auth/local_password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
//! [`ProvenIdentity::local`].
//!
//! The provider only *vouches for* the username; mapping that identity back to
//! a concrete [`UserPrincipal`] (user id, admin flag) is the chain caller's job
//! a concrete [`UserPrincipal`](super::UserPrincipal) (user id, admin flag) is the chain caller's job
//! (see [`super::chain`] mapping helpers). This keeps providers free of the
//! `UserPrincipal` shape, so authorization can change without touching them.
//!
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/authz/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! The [`Authz`](crate::auth::authz::Authz) seam answers `can_read` /
//! `can_write` / `can_delete` from a principal's resolved
//! [`EffectiveSet`](super::EffectiveSet). Computing that set is a DB round trip
//! [`EffectiveSet`]. Computing that set is a DB round trip
//! (`user_role → repository_privilege`), far too costly to run on every
//! download/restore request. This module sits in front of that load with
//! [`forge_cache::Cache`] — **in-process only**
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//! * [`domain`] — neutral types ([`domain::ParsedUpload`]) produced by parsing
//! an uploaded package, the boundary between a concrete format and the rest
//! of the system.
//! * [`format`] — the object-safe [`format::Format`] seam every package format
//! * [`format`](mod@format) — the object-safe [`format::Format`] seam every package format
//! implements (parse, version normalize/compare, id lowercasing).
//! * [`auth`] — principal extraction (token/password/anonymous), client-IP
//! resolution behind trusted proxies, and the authorization seam over the
Expand Down
3 changes: 2 additions & 1 deletion crates/core/src/oidc/claims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
//!
//! The server crate owns the full `OidcConfig` (TOML/env). To avoid a
//! `core → server` dependency inversion, the server maps the claim-relevant
//! fields into this [`ClaimMapping`] when it builds the [`super::OidcProvider`].
//! fields into this [`ClaimMapping`] when it builds its OIDC runtime
//! (`OidcRuntime::build`).
//! The mapping logic and its tests then live entirely in `core`.

use super::jwks::IdTokenClaims;
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/oidc/jwks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! rationale. Everything here is parameterised over an injected
//! [`reqwest::Client`] (so callers pass the no-redirect, https-only client from
//! [`crate::net::ssrf::outbound_metadata_client`]) and a [`forge_cache::Cache`], which is
//! what makes the whole module unit-testable with a [`wiremock`] HTTP server and
//! what makes the whole module unit-testable with a `wiremock` HTTP server and
//! a self-signed token — no real IdP, no real network.
//!
//! Discovery documents and key sets decide which tokens verify, so they are
Expand Down
4 changes: 2 additions & 2 deletions crates/core/src/repo/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! an ordered list of **member** repositories (hosted or proxy). Members carry a
//! per-group `priority` (unique within the group); the highest priority wins
//! ties. Resolution is **identity-aware**: for a calling [`Principal`] `P`, only
//! members `P` may read (per [`authz::can_read`]) are visible. In particular a
//! members `P` may read (per [`Authz::can_read`]) are visible. In particular a
//! mixed-anonymity group serves an anonymous caller only its `anonymous_read`
//! members, while an authenticated caller sees them all.
//!
Expand Down Expand Up @@ -90,7 +90,7 @@
//! past it stops (fail-closed).
//!
//! Identity-aware filtering is applied at **every** level: a member (group or
//! leaf) is descended-into / included only if the principal [`can_read`] it
//! leaf) is descended-into / included only if the principal can read it
//! ([`Authz::can_read`]). The prefer-hosted ownership check above is the one
//! deliberate exception: it only ever *removes* proxy candidates, so it
//! consults every hosted leaf. Being able to read an outer group grants **no**
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/repo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ pub struct SearchParams {
pub include_semver2: bool,
/// Let a proxy forward the search to its upstream's search service, so
/// packages it has not cached yet are found too (see
/// [`proxy`](crate::repo::proxy)). The NuGet search route sets it; the
/// [`proxy`]). The NuGet search route sets it; the
/// browse UI, which lists what Forge holds, does not. Defaults to `true`.
pub include_upstream: bool,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/retention/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! This module is the **format-agnostic** core of retention: it defines the
//! serde [`RetentionPolicy`] stored in `repository.retention_policy` (with
//! [`validate_policy`]) and a **pure** evaluator ([`evaluate`] / [`evaluate_proxy`])
//! [`validate_policy`]) and a **pure** evaluator ([`evaluate()`] / [`evaluate_proxy`])
//! that, given a policy and a component/proxy snapshot, computes the set of
//! versions to delete or unlist. Versions are ordered through the
//! [`crate::format::Format`] seam, never by importing a concrete format.
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/retention/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! The shape:
//! a policy is a list of **rules**; a component is acted on if **any** rule
//! selects it (union), but `keep_last_n` / `keep_n_most_downloaded` protections
//! always win (a protected version is never deleted — see [`crate::retention::evaluate`]).
//! always win (a protected version is never deleted — see [`crate::retention::evaluate()`]).
//! Each rule's protections are computed over the versions that rule's own
//! filters admit (see [`Criteria`]); the protected sets are then unioned.
//!
Expand Down
3 changes: 2 additions & 1 deletion crates/core/src/retention/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
//! 1. parses + validates the claimed repo's `retention_policy`;
//! 2. snapshots its candidate components (and, for proxy repos, cached
//! versions);
//! 3. runs the **pure** [`evaluate`] / [`evaluate_proxy`];
//! 3. runs the **pure** [`evaluate`](super::evaluate()) /
//! [`evaluate_proxy`](super::evaluate_proxy);
//! 4. [`apply`]s the decisions — each in its own tx, decref + `blob_outbox`
//! only (no direct S3 delete);
//! 5. records the run summary + schedules the next run on the ledger: at the
Expand Down
2 changes: 1 addition & 1 deletion crates/db/src/authz_dao.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub const READER_ROLE: &str = "reader";

/// Shared shape of the effective-set query rows (user + anonymous variants).
/// The two `sqlx::query!` calls generate distinct anonymous structs with the
/// same columns; [`into_query_row!`] converts each into this adapter so
/// same columns; `into_query_row!` converts each into this adapter so
/// [`map_grant_row`] is reused across both call sites.
struct EffectiveQueryRow {
level: String,
Expand Down
5 changes: 3 additions & 2 deletions crates/db/src/validated_credential_dao.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,9 @@ pub async fn list_auth_successes(pool: &PgPool, username: &str) -> sqlx::Result<
/// Revoke every cached verdict for a username (optionally scoped to one
/// provider): the admin "force re-auth user X" / deactivate-user path. Returns
/// the number of rows deleted (verdicts plus any in-flight singleflight
/// claims, whose winners are thereby fenced — see [`finalize_claim`]). The caller must also drop the matching cache
/// entries via the [`forge_cache`] invalidation backplane.
/// claims, whose winners are thereby fenced — see [`finalize_claim`]). The
/// caller must also drop the matching cache entries via the `forge-cache`
/// invalidation backplane.
pub async fn revoke_by_username(
pool: &PgPool,
username: &str,
Expand Down
8 changes: 4 additions & 4 deletions crates/e2e/src/mock_oidc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,10 @@ admin_claim = "roles"
claims
}

/// Rotate the signing key: snapshot the **old** signer (so a test can still
/// sign a token with the now-retired key), publish only the **new** key in
/// the JWKS, and make it the active signer. Returns the retired signer's
/// behaviour through [`Self::set_next_id_token_with_retired_key`].
/// Rotate the signing key: publish only the **new** key in the JWKS and make
/// it the active signer, so the old `kid` is a miss in the JWKS from now on.
/// A token signed by a key the JWKS does not hold is
/// [`Self::set_next_id_token_with_unknown_key`]'s.
pub async fn rotate_key(&self, new_kid: &str) {
let new_signer = Signer::new(new_kid);
let mut inner = self.inner.lock().await;
Expand Down
2 changes: 1 addition & 1 deletion crates/formats/docker/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Docker / OCI registries for Forge, as a format plugin: the
//! distribution-spec v1.1 API under `/v2`, over the host's image store.
//!
//! * [`path`], [`digest`], [`reference`]: the protocol's paths and the
//! * [`path`], [`digest`], [`reference`](mod@reference): the protocol's paths and the
//! grammar of its names, references and digests;
//! * [`manifest`]: Docker v2 schema 2 manifests and manifest lists, OCI
//! image manifests and indexes, artifacts with `artifactType` and
Expand Down
7 changes: 4 additions & 3 deletions crates/formats/nuget/src/nuspec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! `.nupkg` (zip) and `.nuspec` (XML) parsing into a [`ParsedNupkg`].
//! `.nupkg` (zip) and `.nuspec` (XML) parsing into a
//! [`ParsedNupkg`](crate::model::ParsedNupkg).
//!
//! A `.nupkg` is an OPC zip. We locate the single top-level `*.nuspec`
//! manifest, read it under strict zip-bomb limits, and parse it
Expand All @@ -7,7 +8,7 @@
//!
//! Only the manifest is decompressed; the `<id>`, `<version>`, and
//! `<dependencies>` groups are extracted. The raw manifest bytes are carried in
//! [`ParsedNupkg::nuspec_bytes`] so the server can store and re-serve them.
//! [`ParsedNupkg::nuspec_bytes`](crate::model::ParsedNupkg::nuspec_bytes) so the server can store and re-serve them.

use std::io::{BufReader, Read, SeekFrom};

Expand All @@ -20,7 +21,7 @@ use crate::model::{
};
use crate::version;

/// Parse a `.nupkg` from a seekable reader into a [`ParsedNupkg`].
/// Parse a `.nupkg` from a seekable reader into a [`ParsedNupkg`](ParsedUpload).
pub fn parse_nupkg(
file: &mut dyn ReadSeek,
limits: &NupkgLimits,
Expand Down
6 changes: 4 additions & 2 deletions crates/server/src/browse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
//! * version visibility, group member resolution, listed/prerelease filtering,
//! and proxy **package-delay** gating all come for free from
//! [`AppState::read_ops_for`] (the package detail's
//! [`AppState::cached_read_ops_for`]) + [`RepoReadOps`] — the same ops the
//! [`AppState::cached_read_ops_for`]) +
//! [`RepoReadOps`](forge_core::repo::RepoReadOps) — the same ops the
//! repository dispatcher uses, so browse can never surface a version the
//! protocol would hide.
//!
Expand Down Expand Up @@ -251,7 +252,8 @@ pub struct BrowseResult {
/// `GET /browse/packages` — cross-repository search.
///
/// Iterates every repository the principal may read (honoring
/// `anonymous_read`), runs [`RepoReadOps::search`] on each, merges the hits,
/// `anonymous_read`), runs
/// [`RepoReadOps::search`](forge_core::repo::RepoReadOps::search) on each, merges the hits,
/// then sorts + paginates the merged set. A `repo` filter restricts to a single
/// repository, which then pages in its backend. `total` is the sum of the
/// backends' `total_hits` (already filtered by the caller's package selector).
Expand Down
9 changes: 5 additions & 4 deletions crates/server/src/me.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
//! `GET /admin/me` — the authenticated principal's identity + capability flags.
//!
//! This endpoint is mounted **outside** the admin module's [`require_admin`]
//! gate: it is reachable by any *authenticated* user (admin or not) so the SPA
//! can decide which navigation/management screens to show. It still requires an
//! authenticated principal — an anonymous or unauthenticated caller gets `401`.
//! This endpoint is mounted **outside** the admin module's
//! [`require_admin`](crate::admin::require_admin) gate: it is reachable by any
//! *authenticated* user (admin or not) so the SPA can decide which
//! navigation/management screens to show. It still requires an authenticated
//! principal — an anonymous or unauthenticated caller gets `401`.
//!
//! ## Capabilities
//! `is_admin` and the capability list come from the **effective RBAC** verdict
Expand Down
14 changes: 14 additions & 0 deletions web/knip.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// knip: files, exports and dependencies nothing uses (`pnpm run knip`, CI's
// web job). Its plugins find the entry files in index.html and the Vite,
// Vitest and Playwright configs.
{
"$schema": "https://unpkg.com/knip@6/schema.json",
// The TanStack Router plugin loads the route tree from src/routes.ts by
// its path (vite.config.ts).
"entry": ["src/routes.ts"],
// The stylesheets too, for the packages they import (`@import
// 'tailwindcss'`).
"project": ["src/**/*.{ts,tsx,css}", "e2e/**/*.ts"],
// An export the module itself uses is no dead code, only a wider scope.
"ignoreExportsUsedInFile": true
}
4 changes: 3 additions & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"e2e:ui": "playwright test --ui",
"e2e:list": "playwright test --list",
"e2e:tsc": "tsc -p e2e/tsconfig.json",
"e2e:install": "playwright install --with-deps chromium"
"e2e:install": "playwright install --with-deps chromium",
"knip": "knip"
},
"dependencies": {
"@fontsource-variable/ibm-plex-sans": "5.3.0",
Expand All @@ -41,6 +42,7 @@
"@types/react-dom": "19.3.0",
"@vitejs/plugin-react": "6.1.2",
"jsdom": "30.1.3",
"knip": "6.43.0",
"tailwindcss": "4.3.3",
"typescript": "7.0.2",
"vite": "8.3.5",
Expand Down
Loading
Loading