mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
fix(security): reconcile served refs with authorized events
Motivation: the v3 storage migration preserves legacy refs exactly, so structural Git integrity alone cannot detect refs written through the pre-v3 GRASP-06 path traversal. Operators need an online, post-migration answer without extending the production outage. Approach: add a second startup pass that derives owner refs from the NIP-01-preferred State of the confirmed maintainer set and PR refs from accepted PR/PR Update events, including exact GRASP-06 and active-purgatory scoping. Fetch missing expected objects through the hardened repair path, repair unambiguous drift, and preserve unexplained refs with bounded manual-inspection logs. Correctness: authoritative events are refreshed while holding the family lease, ref updates use compare-and-swap, and anything changed since the initial online snapshot is left untouched. This assumes the accepted event database and existing membership/GRASP-06 predicates are authoritative. The offline migration is deliberately unchanged, and unexplained PR refs are not auto-deleted because they may be evidence. Validation: cargo fmt --all -- --check; cargo clippy --workspace --all-targets -- -D warnings; cargo test --locked; cargo test --lib --locked (892 passed after the final race guard).
This commit is contained in:
+9
-1
@@ -80,7 +80,15 @@ Expect a bit of downtime as a git data migraiton is performed on startup. The la
|
||||
repositories accessible to the service. With GRASP-06 enabled, crafted PR
|
||||
submissions could also write Git objects and PR refs into another hosted
|
||||
repository without its maintainer authorization. Operators must upgrade
|
||||
to v3.0.0; GRASP-06 operators should review hosted repository integrity.
|
||||
to v3.0.0. On every v3 startup, a non-blocking authorization-integrity pass
|
||||
compares every served branch, tag, `HEAD`, and `refs/nostr/*` ref with the
|
||||
accepted State, PR, and PR Update events (including precisely scoped
|
||||
in-flight events). It repairs unambiguous differences and emits
|
||||
`manual_inspection=true` errors without deleting unexplained PR refs that
|
||||
may be evidence. GRASP-06 operators must check those logs and the terminal
|
||||
`Git authorization-integrity startup pass completed` summary; any non-zero
|
||||
`manual_inspection` or `failed` count means the named repository still
|
||||
requires review.
|
||||
|
||||
### Added
|
||||
|
||||
|
||||
@@ -90,6 +90,11 @@ runtime:
|
||||
identity stays local so the relay's existence is never advertised
|
||||
- Atomically checkpoint purgatory and rejected-event recovery state every 60
|
||||
seconds without consuming the checkpoint during restore
|
||||
- Start non-blocking storage-integrity and authorization-integrity passes after
|
||||
database initialization. The former checks family objects and thin-view
|
||||
wiring; the latter reconciles each served ref against accepted State, PR,
|
||||
and PR Update events, auto-repairing only unambiguous differences and
|
||||
logging preserved evidence for manual inspection
|
||||
- Serve HTTP + WebSocket until a caller-supplied shutdown future
|
||||
resolves, then stop background mutation, persist a final state snapshot
|
||||
(purgatory and rejected-events cache), and clean up placeholder refs
|
||||
|
||||
@@ -382,14 +382,15 @@ View lifecycle locks remain responsible for “may this path be removed?” The
|
||||
family lock is responsible for “is this object inventory and manifest update
|
||||
atomic?” Neither lock grants authorization.
|
||||
|
||||
## Integrity and healing
|
||||
## Storage and authorization integrity
|
||||
|
||||
Integrity is defined for an object-format/identifier family, not for one
|
||||
owner path. A pass checks the family pack set and object graph, then checks
|
||||
every owner and `/prs/` view with the same identifier for the correct alternate
|
||||
and for refs whose targets are available in the family. Multiple independent
|
||||
histories in one identifier are valid, and unreachable objects are not an
|
||||
error: retained delete-state and rollback data intentionally remain present.
|
||||
Storage integrity is defined for an object-format/identifier family, not for
|
||||
one owner path. The storage pass checks the family pack set and object graph,
|
||||
then checks every owner and `/prs/` view with the same identifier for the
|
||||
correct alternate and for refs whose targets are available in the family.
|
||||
Multiple independent histories in one identifier are valid, and unreachable
|
||||
objects are not an error: retained delete-state and rollback data intentionally
|
||||
remain present.
|
||||
|
||||
The relay starts one non-blocking pass after migration, database
|
||||
initialization, and construction of the hardened outbound Git client. Broken
|
||||
@@ -401,6 +402,33 @@ integrity check again. An unresolved family produces an `ERROR` log containing
|
||||
bounded counts and OID/diagnostic samples; it does not make availability depend
|
||||
on remote servers.
|
||||
|
||||
Authorization integrity is the second, view-scoped layer. It derives each
|
||||
owner's branch, tag, and `HEAD` set from the NIP-01-preferred accepted State
|
||||
event published by that owner's confirmed maintainer set. It derives
|
||||
`refs/nostr/<event-id>` from accepted PR and PR Update events using the same
|
||||
maintainer-overlap propagation rule as normal event processing. GRASP-06 views
|
||||
also require the signer and this relay's clone URL to name that exact
|
||||
submitter/identifier coordinate. Exactly scoped active purgatory entries are
|
||||
recognized so an in-flight push is not mistaken for corruption.
|
||||
|
||||
The pass creates or updates missing/wrong authorized refs once their objects
|
||||
are available, deletes stale State-governed branch and tag refs, and repairs
|
||||
`HEAD`. Before giving up on a missing expected object it uses the same accepted
|
||||
clone URLs and hardened fetch path as storage healing. It does not delete an
|
||||
unexplained PR or unknown-namespace ref: that ref may be evidence of a past
|
||||
authorization bypass. Instead it emits a bounded, structured `ERROR` with
|
||||
`manual_inspection=true`, the view and ref, and actual/expected targets. An
|
||||
authorized State still in purgatory also defers State mutation and is reported
|
||||
for inspection rather than racing event promotion.
|
||||
|
||||
Both startup passes run in the background after database initialization. They
|
||||
do not extend the offline migration window or make relay availability depend
|
||||
on a remote Git server. Their stable terminal log messages are `Git
|
||||
storage-integrity startup pass completed` and `Git authorization-integrity
|
||||
startup pass completed`. Because the authorization pass is online, it refreshes
|
||||
the accepted events immediately before mutation and refuses to overwrite any
|
||||
ref that changed after its initial snapshot.
|
||||
|
||||
This is also the migration repair path. Migration remains a deterministic,
|
||||
offline conversion that preserves every Git-readable object and the exact
|
||||
legacy refs. Once those paths are thin family views, the ordinary family pass
|
||||
@@ -408,7 +436,7 @@ can heal pre-existing missing objects. Unindexed legacy packs are preserved
|
||||
under `.grasp/migration/unindexed-packs/` when their backup is retired; there
|
||||
is no separate legacy repair subsystem.
|
||||
|
||||
Operators can queue the same identifier-scoped check in the live process:
|
||||
Operators can queue an identifier-scoped storage check in the live process:
|
||||
|
||||
```console
|
||||
ngit-grasp integrity-check --identifier example
|
||||
|
||||
@@ -51,18 +51,42 @@ crash-safe, and safe to resume by restarting the same v3 release. It:
|
||||
|
||||
Progress is stored below `.grasp/migration/`; the completed layout is marked by
|
||||
`.grasp/storage-version`. Do not edit these files while the service is running.
|
||||
A later v3 restart sees that completed marker and does not repeat the offline
|
||||
v2 conversion; the production-scale 51-minute cost applies to the first
|
||||
v2-to-v3 migration, not every v3 deployment.
|
||||
|
||||
Confirm that the service reaches its normal listening state, then check the
|
||||
terminal integrity summary:
|
||||
Confirm that the service reaches its normal listening state, then follow the
|
||||
logs until both terminal integrity summaries appear:
|
||||
|
||||
```text
|
||||
Git identifier-family integrity startup pass completed
|
||||
Git storage-integrity startup pass completed
|
||||
Git authorization-integrity startup pass completed
|
||||
```
|
||||
|
||||
An `unresolved` or `failed` count above zero has a corresponding `ERROR` naming
|
||||
the identifier. The server has already attempted automatic repair. A retained
|
||||
backup protects an unhealthy family's legacy data, and a legacy shallow view
|
||||
continues serving at its pre-upgrade level rather than being made less usable.
|
||||
These passes are non-blocking: the relay is online while they inspect and heal
|
||||
the migrated views. In the storage summary, an `unresolved` or `failed` count
|
||||
above zero has a corresponding `ERROR` naming the identifier. In the
|
||||
authorization summary, a `manual_inspection` or `failed` count above zero has
|
||||
a corresponding `ERROR` naming the exact view, ref, actual target, expected
|
||||
target, and reason. The server has already attempted safe automatic repair.
|
||||
It deliberately preserves unexplained `refs/nostr/*` and unknown-namespace
|
||||
refs as evidence instead of guessing that deletion is safe.
|
||||
|
||||
The authorization pass treats the accepted event database as authoritative:
|
||||
|
||||
- the latest State event from the owner's confirmed maintainer set defines
|
||||
all and only `refs/heads/*`, `refs/tags/*`, and `HEAD`;
|
||||
- accepted PR and PR Update events define `refs/nostr/<event-id>` in every
|
||||
owner view selected by the existing maintainer-overlap rules;
|
||||
- a GRASP-06 contributor view additionally requires the event signer, clone
|
||||
URL, and repository identifier to match that exact `/prs/` coordinate;
|
||||
- precisely scoped active purgatory entries are tolerated as in-flight state.
|
||||
Legacy unscoped placeholders are preserved but reported because they cannot
|
||||
prove which owner and identifier originally received the push.
|
||||
|
||||
A retained backup protects an unhealthy family's legacy data, and a legacy
|
||||
shallow view continues serving at its pre-upgrade level rather than being made
|
||||
less usable.
|
||||
|
||||
## Roll back
|
||||
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -165,6 +165,7 @@ pub fn spawn_integrity_worker(
|
||||
) -> JoinHandle<()> {
|
||||
tokio::spawn(async move {
|
||||
run_startup_pass(&storage, source.as_ref()).await;
|
||||
super::authorization_integrity::run_startup_pass(&storage, source.as_ref()).await;
|
||||
let first = tokio::time::Instant::now() + REQUEST_POLL_INTERVAL;
|
||||
let mut interval = tokio::time::interval_at(first, REQUEST_POLL_INTERVAL);
|
||||
loop {
|
||||
@@ -178,13 +179,13 @@ async fn run_startup_pass<S: FamilyRepairSource + ?Sized>(storage: &LocalGitStor
|
||||
let families = match discover_families(storage) {
|
||||
Ok(families) => families,
|
||||
Err(error) => {
|
||||
error!(%error, "Git identifier-family integrity startup discovery failed");
|
||||
error!(%error, "Git storage-integrity startup discovery failed");
|
||||
return;
|
||||
}
|
||||
};
|
||||
info!(
|
||||
families = families.len(),
|
||||
"Git identifier-family integrity startup pass started"
|
||||
"Git storage-integrity startup pass started"
|
||||
);
|
||||
let mut stats = PassStats::default();
|
||||
for key in families {
|
||||
@@ -197,7 +198,7 @@ async fn run_startup_pass<S: FamilyRepairSource + ?Sized>(storage: &LocalGitStor
|
||||
repaired = stats.repaired,
|
||||
unresolved = stats.unresolved,
|
||||
failed = stats.failed,
|
||||
"Git identifier-family integrity startup pass completed"
|
||||
"Git storage-integrity startup pass completed"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -651,7 +652,7 @@ pub(crate) fn family_contains_closure(family: &Path, refs_source: &Path) -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn discover_views(storage: &LocalGitStorage, key: &FamilyKey) -> Result<Vec<PathBuf>> {
|
||||
pub(crate) fn discover_views(storage: &LocalGitStorage, key: &FamilyKey) -> Result<Vec<PathBuf>> {
|
||||
let mut views = Vec::new();
|
||||
let entries = match std::fs::read_dir(storage.git_data_path()) {
|
||||
Ok(entries) => entries,
|
||||
@@ -699,7 +700,7 @@ fn maybe_add_view(directory: &Path, key: &FamilyKey, views: &mut Vec<PathBuf>) -
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn list_refs(repo: &Path) -> Result<Vec<(String, String)>> {
|
||||
pub(crate) fn list_refs(repo: &Path) -> Result<Vec<(String, String)>> {
|
||||
let output = Command::new("git")
|
||||
.args(["for-each-ref", "--format=%(refname)%00%(objectname)"])
|
||||
.current_dir(repo)
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
//! - `POST /<npub>/<identifier>.git/git-receive-pack` - Push operation
|
||||
|
||||
pub mod authorization;
|
||||
pub mod authorization_integrity;
|
||||
pub mod handlers;
|
||||
pub mod integrity;
|
||||
pub mod migration;
|
||||
|
||||
+15
-9
@@ -1351,25 +1351,23 @@ async fn process_purgatory_state_events(
|
||||
result
|
||||
}
|
||||
|
||||
/// Check if a state event is the latest authorized state for a given maintainer set.
|
||||
/// Select the latest authorized State event for a given maintainer set.
|
||||
///
|
||||
/// Only considers states already in the database, not other purgatory states.
|
||||
///
|
||||
/// # Arguments
|
||||
/// * `state` - The state event to check
|
||||
/// * `maintainers` - The set of authorized maintainers for the owner
|
||||
/// * `db_states` - State events from the database
|
||||
///
|
||||
/// # Returns
|
||||
/// true if this state is the latest (or equal latest) among all authorized states in the DB
|
||||
fn is_latest_authorized_state(
|
||||
state: &RepositoryState,
|
||||
/// The NIP-01-preferred State: newest timestamp, then lowest event ID.
|
||||
pub fn latest_authorized_state<'a>(
|
||||
maintainers: &[String],
|
||||
db_states: &[RepositoryState],
|
||||
) -> bool {
|
||||
db_states: &'a [RepositoryState],
|
||||
) -> Option<&'a RepositoryState> {
|
||||
// Find the NIP-01-preferred authorized state from the database: newest
|
||||
// timestamp wins, and the lowest event ID wins an equal-timestamp tie.
|
||||
let latest_db_state = db_states
|
||||
db_states
|
||||
.iter()
|
||||
.filter(|s| maintainers.contains(&s.event.pubkey.to_hex()))
|
||||
.max_by(|a, b| {
|
||||
@@ -1379,7 +1377,15 @@ fn is_latest_authorized_state(
|
||||
.created_at
|
||||
.cmp(&b.event.created_at)
|
||||
.then_with(|| b.event.id.cmp(&a.event.id))
|
||||
});
|
||||
})
|
||||
}
|
||||
|
||||
fn is_latest_authorized_state(
|
||||
state: &RepositoryState,
|
||||
maintainers: &[String],
|
||||
db_states: &[RepositoryState],
|
||||
) -> bool {
|
||||
let latest_db_state = latest_authorized_state(maintainers, db_states);
|
||||
|
||||
match latest_db_state {
|
||||
None => true, // No other states exist in DB, this is the latest
|
||||
|
||||
@@ -1121,6 +1121,19 @@ impl Purgatory {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Snapshot every active PR event and git-data-first placeholder.
|
||||
///
|
||||
/// The authorization-integrity pass needs the event ID (the map key) as
|
||||
/// well as the entry so it can distinguish a legitimate in-flight ref
|
||||
/// from an unexplained served ref. Keeping this crate-private avoids
|
||||
/// exposing the purgatory's internal indexing as public API.
|
||||
pub(crate) fn pr_entries_for_integrity(&self) -> Vec<(String, PrPurgatoryEntry)> {
|
||||
self.pr_events
|
||||
.iter()
|
||||
.map(|entry| (entry.key().clone(), entry.value().clone()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Remove expired entries from purgatory.
|
||||
///
|
||||
/// Should be called periodically (every 60 seconds) by background task to clean up
|
||||
|
||||
@@ -360,6 +360,71 @@ impl RealSyncContext {
|
||||
urls.dedup();
|
||||
Ok(urls)
|
||||
}
|
||||
|
||||
pub(crate) async fn accepted_repository_data_for_integrity(
|
||||
&self,
|
||||
identifier: &str,
|
||||
) -> Result<RepositoryData> {
|
||||
crate::git::authorization::fetch_repository_data_excluding_purgatory(
|
||||
&self.database,
|
||||
identifier,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Accepted PR and PR-update events used to reconstruct the authorized
|
||||
/// `refs/nostr/*` surface during the startup integrity pass.
|
||||
pub(crate) async fn accepted_pr_events_for_integrity(
|
||||
&self,
|
||||
) -> Result<Vec<nostr_sdk::prelude::Event>> {
|
||||
use nostr_sdk::prelude::{Filter, Kind};
|
||||
|
||||
self.database
|
||||
.query(Filter::new().kinds([Kind::GitPullRequest, Kind::GitPullRequestUpdate]))
|
||||
.await
|
||||
.map(|events| events.into_iter().collect())
|
||||
.map_err(|error| anyhow::anyhow!("Database query failed: {error}"))
|
||||
}
|
||||
|
||||
/// Revalidate a bounded set of PR IDs immediately before ref mutation.
|
||||
/// Deleted events disappear from this result, preventing a long-running
|
||||
/// online pass from recreating refs from its older startup snapshot.
|
||||
pub(crate) async fn accepted_pr_events_by_id_for_integrity(
|
||||
&self,
|
||||
ids: &[nostr_sdk::prelude::EventId],
|
||||
) -> Result<Vec<nostr_sdk::prelude::Event>> {
|
||||
use nostr_sdk::prelude::{Filter, Kind};
|
||||
|
||||
if ids.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
self.database
|
||||
.query(
|
||||
Filter::new()
|
||||
.ids(ids.iter().copied())
|
||||
.kinds([Kind::GitPullRequest, Kind::GitPullRequestUpdate]),
|
||||
)
|
||||
.await
|
||||
.map(|events| events.into_iter().collect())
|
||||
.map_err(|error| anyhow::anyhow!("Database query failed: {error}"))
|
||||
}
|
||||
|
||||
pub(crate) fn pending_pr_entries_for_integrity(
|
||||
&self,
|
||||
) -> Vec<(String, crate::purgatory::PrPurgatoryEntry)> {
|
||||
self.purgatory.pr_entries_for_integrity()
|
||||
}
|
||||
|
||||
pub(crate) fn pending_state_events_for_integrity(
|
||||
&self,
|
||||
identifier: &str,
|
||||
) -> Vec<crate::purgatory::StatePurgatoryEntry> {
|
||||
self.purgatory.find_state(identifier)
|
||||
}
|
||||
|
||||
pub(crate) fn service_address_for_integrity(&self) -> Option<&str> {
|
||||
self.our_domain_value.as_deref()
|
||||
}
|
||||
}
|
||||
|
||||
const MISS_MEMO_TTL: Duration = Duration::from_secs(30 * 60);
|
||||
|
||||
+1
-1
@@ -456,7 +456,7 @@ impl RelayServer {
|
||||
git_storage,
|
||||
sync_ctx.clone(),
|
||||
));
|
||||
info!("Git identifier-family integrity worker started");
|
||||
info!("Git storage and authorization-integrity worker started");
|
||||
|
||||
// Create throttle manager for rate limiting remote git servers
|
||||
// Default: 5 concurrent requests per domain, 60 requests per minute per domain
|
||||
|
||||
Reference in New Issue
Block a user