fix(nostr): reject PR events missing commit metadata as invalid

Production sync repeatedly retried immutable PR events without a c tag because
the Git validation error was classified as a retryable server failure.

Validate required commit metadata before Git access and return an invalid
rejection for PR and PR-update events. Sync's existing terminal accounting then
stops hydration retries. Reuse the metadata extraction in the Git policy.

Only absent commit values are reclassified. This assumes signed metadata
cannot be repaired by fetching another copy; unavailable Git objects still
enter purgatory and actual Git/database faults remain retryable. Commit hash
syntax and other PR metadata validation are deliberately unchanged.

Validation: the regression covers both event kinds, missing and valueless
tags, synced and direct delivery, and valid metadata awaiting Git data.
All 924 library tests and the PR hosting and purgatory integration suites
passed, along with all-target Clippy with warnings denied and formatting.

Assisted-by: GPT-6
This commit is contained in:
DanConwayDev
2026-09-23 15:07:19 +00:00
parent 00911d8914
commit 56889bfc1d
4 changed files with 102 additions and 9 deletions
+3
View File
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ### Fixed
- Reject PR and PR-update events missing commit metadata as terminal invalid
events, avoiding repeated recovery attempts for immutable malformed data.
- Retire incomplete historic batches when transient requests time out or close - Retire incomplete historic batches when transient requests time out or close
before EOSE, and bound SDK subscription retention with an auto-close deadline. before EOSE, and bound SDK subscription retention with an auto-close deadline.
+6
View File
@@ -398,6 +398,12 @@ The purgatory system solves two related problems:
**Design Document**: See [`purgatory-design.md`](purgatory-design.md) for complete design specifications. **Design Document**: See [`purgatory-design.md`](purgatory-design.md) for complete design specifications.
PR and PR-update events must include a commit value in their signed `c` tag.
Missing metadata is rejected as `invalid` before Git checks, so sync treats the
event as terminally accounted rather than repeatedly retrying an apparent
server failure. Events with commit metadata but unavailable Git objects remain
eligible for purgatory; Git and database failures remain retryable.
#### Architecture #### Architecture
```rust ```rust
+82
View File
@@ -551,6 +551,11 @@ impl Nip34WritePolicy {
/// * `event` - The PR event to validate /// * `event` - The PR event to validate
/// * `is_synced` - True if this event came from proactive sync (vs user-submitted) /// * `is_synced` - True if this event came from proactive sync (vs user-submitted)
async fn handle_pr_event(&self, event: &Event, is_synced: bool) -> WritePolicyResult { async fn handle_pr_event(&self, event: &Event, is_synced: bool) -> WritePolicyResult {
// Missing signed metadata cannot be repaired by fetching Git data or
// retrying another relay. Keep actual Git/database faults retryable.
if PrEventPolicy::commit(event).is_none() {
return reject_invalid(format!("PR event {} has no 'c' tag", event.id));
}
let event_id_str = event.id.to_bech32().unwrap_or_else(|_| event.id.to_hex()); let event_id_str = event.id.to_bech32().unwrap_or_else(|_| event.id.to_hex());
// duplicate check in purgatory // duplicate check in purgatory
@@ -1176,3 +1181,80 @@ pub async fn create_relay(
}, },
}) })
} }
#[cfg(test)]
mod pr_metadata_tests {
use super::*;
use crate::grasp06::receive::RepoInitLocks;
use std::path::PathBuf;
#[tokio::test]
async fn missing_pr_commit_is_invalid_but_missing_git_data_enters_purgatory() {
let directory = tempfile::tempdir().unwrap();
let mut config = Config::for_testing();
config.grasp06_enable = true;
config.git_data_path = directory.path().join("git").to_string_lossy().into_owned();
config.relay_data_path = directory
.path()
.join("relay")
.to_string_lossy()
.into_owned();
let purgatory = Arc::new(crate::purgatory::Purgatory::new(PathBuf::from(
&config.git_data_path,
)));
let runtime = create_relay(&config, purgatory, RepoInitLocks::default(), None)
.await
.unwrap();
let keys = Keys::generate();
let tags = vec![
Tag::custom("a", [format!("30617:{}:test", keys.public_key().to_hex())]),
Tag::custom(
"clone",
[format!(
"https://{}/prs/{}/test.git",
config.service_address(),
keys.public_key().to_bech32().unwrap()
)],
),
];
for kind in [Kind::GitPullRequest, Kind::GitPullRequestUpdate] {
for empty_tag in [false, true] {
let mut event_tags = tags.clone();
if empty_tag {
event_tags.push(Tag::custom("c", Vec::<String>::new()));
}
let event = EventBuilder::new(kind, "missing commit")
.tags(event_tags)
.finalize(&keys)
.unwrap();
for synced in [false, true] {
let result = runtime.write_policy.handle_pr_event(&event, synced).await;
assert!(
matches!(
result,
WritePolicyResult::Reject {
prefix: MachineReadablePrefix::Invalid,
status: false,
..
}
),
"missing immutable metadata must be a terminal validation failure"
);
}
}
let event = EventBuilder::new(kind, "awaiting Git data")
.tags(tags.clone())
.tag(Tag::custom(
"c",
["1111111111111111111111111111111111111111"],
))
.finalize(&keys)
.unwrap();
let result = runtime.write_policy.handle_pr_event(&event, true).await;
assert!(
matches!(result, WritePolicyResult::Reject { status: true, .. }),
"valid metadata with unavailable Git objects must remain eligible for purgatory"
);
}
}
}
+11 -9
View File
@@ -30,6 +30,16 @@ impl PrEventPolicy {
} }
} }
/// Extract required commit metadata separately from fallible Git I/O.
pub(crate) fn commit(event: &Event) -> Option<String> {
event.tags.iter().find_map(|tag| {
let values = tag.as_slice();
(values.first().map(String::as_str) == Some("c"))
.then(|| values.get(1).cloned())
.flatten()
})
}
/// Check if git data exists for a PR event /// Check if git data exists for a PR event
/// ///
/// This unified method checks for git data existence and handles: /// This unified method checks for git data existence and handles:
@@ -46,15 +56,7 @@ impl PrEventPolicy {
pub async fn git_data_check(&self, event: &Event) -> Result<bool> { pub async fn git_data_check(&self, event: &Event) -> Result<bool> {
let event_id = event.id.to_hex(); let event_id = event.id.to_hex();
// Extract the `c` tag (commit hash) from the PR event let commit = Self::commit(event);
let commit = event.tags.iter().find_map(|tag| {
let tag_vec = tag.clone().to_vec();
if tag_vec.len() >= 2 && tag_vec[0] == "c" {
Some(tag_vec[1].clone())
} else {
None
}
});
let commit = match commit { let commit = match commit {
Some(c) => c, Some(c) => c,