fix: broadcast goal progress on review log, fix goal handler security gaps
Some checks failed
CI / Check / Test (push) Has been cancelled

- Broadcast GoalUpdated AP note after ReviewLogged so federated goal
  progress reflects the new review count without requiring a manual goal edit
- Add attribution check in GoalObjectHandler::on_update (mirrors
  review_handler) to prevent any remote actor from overwriting another's goal
- Implement on_actor_removed in GoalObjectHandler via new
  RemoteGoalRepository::remove_all_by_actor — remote goals were never
  cleaned up when an actor unfollowed or was deleted
- Add remove_all_by_actor to SQLite, Postgres, Noop, and test Panic impls
This commit is contained in:
2026-06-10 02:40:25 +02:00
parent 05d062f4e0
commit d389e26e39
7 changed files with 78 additions and 2 deletions

View File

@@ -1,4 +1,5 @@
use async_trait::async_trait;
use chrono::Datelike;
use domain::ports::EventHandler;
use domain::{
errors::DomainError,
@@ -172,6 +173,9 @@ impl ActivityPubEventHandler {
.broadcast_create_note(user_id.value(), json, ApVisibility::Public, vec![])
.await?;
let year = review.watched_at().year() as u16;
self.broadcast_goal_progress_update(user_id, year).await?;
Ok(())
}
@@ -334,6 +338,45 @@ impl ActivityPubEventHandler {
Ok(())
}
async fn broadcast_goal_progress_update(
&self,
user_id: &UserId,
year: u16,
) -> anyhow::Result<()> {
if !self
.content_query
.get_user_federate_goals(user_id)
.await
.unwrap_or(false)
{
return Ok(());
}
let Some((goal, current)) = self
.content_query
.get_goal_with_progress(user_id, year)
.await
.ok()
.flatten()
else {
return Ok(());
};
let ap_id = goal_url(&self.base_url, user_id.value(), year);
let actor = actor_url(&self.base_url, user_id.value());
let obj = goal_to_ap_object(
ap_id,
actor,
year,
goal.target_count(),
current,
&self.base_url,
);
let json = serde_json::to_value(obj)?;
self.ap_service
.broadcast_update_note(user_id.value(), json, ApVisibility::Public, vec![])
.await?;
Ok(())
}
async fn broadcast_goal(
&self,
user_id: &UserId,

View File

@@ -42,7 +42,7 @@ impl ApObjectHandler for GoalObjectHandler {
async fn on_update(
&self,
ap_id: &Url,
_actor_url: &Url,
actor_url: &Url,
object: serde_json::Value,
) -> anyhow::Result<()> {
let obj: GoalObject = match serde_json::from_value(object) {
@@ -52,6 +52,9 @@ impl ApObjectHandler for GoalObjectHandler {
return Ok(());
}
};
if obj.attributed_to != *actor_url {
anyhow::bail!("goal Update actor does not match object attributed_to");
}
self.remote_goal_repo
.update_by_ap_id(ap_id.as_str(), obj.goal_target, obj.goal_current)
.await?;
@@ -67,7 +70,10 @@ impl ApObjectHandler for GoalObjectHandler {
Ok(())
}
async fn on_actor_removed(&self, _actor_url: &Url) -> anyhow::Result<()> {
async fn on_actor_removed(&self, actor_url: &Url) -> anyhow::Result<()> {
self.remote_goal_repo
.remove_all_by_actor(actor_url.as_str())
.await?;
Ok(())
}

View File

@@ -73,6 +73,16 @@ impl RemoteGoalRepository for PostgresRemoteGoalRepository {
Ok(())
}
async fn remove_all_by_actor(&self, actor_url: &str) -> Result<(), DomainError> {
sqlx::query("DELETE FROM remote_goals WHERE actor_url = $1")
.bind(actor_url)
.execute(&self.pool)
.await
.map_err(Self::map_err)?;
Ok(())
}
async fn get_by_actor_url(&self, actor_url: &str) -> Result<Vec<RemoteGoalEntry>, DomainError> {
let rows = sqlx::query(
"SELECT ap_id, actor_url, year, target_count, current_count, \

View File

@@ -69,6 +69,16 @@ impl RemoteGoalRepository for SqliteRemoteGoalRepository {
Ok(())
}
async fn remove_all_by_actor(&self, actor_url: &str) -> Result<(), DomainError> {
sqlx::query("DELETE FROM remote_goals WHERE actor_url = ?")
.bind(actor_url)
.execute(&self.pool)
.await
.map_err(Self::map_err)?;
Ok(())
}
async fn get_by_actor_url(&self, actor_url: &str) -> Result<Vec<RemoteGoalEntry>, DomainError> {
let rows = sqlx::query(
"SELECT ap_id, actor_url, year, target_count, current_count, received_at \

View File

@@ -444,6 +444,7 @@ pub trait RemoteGoalRepository: Send + Sync {
current: u32,
) -> Result<(), DomainError>;
async fn remove_by_ap_id(&self, ap_id: &str, actor_url: &str) -> Result<(), DomainError>;
async fn remove_all_by_actor(&self, actor_url: &str) -> Result<(), DomainError>;
async fn get_by_actor_url(&self, actor_url: &str) -> Result<Vec<RemoteGoalEntry>, DomainError>;
}

View File

@@ -182,6 +182,9 @@ impl crate::ports::RemoteGoalRepository for NoopRemoteGoalRepository {
async fn remove_by_ap_id(&self, _: &str, _: &str) -> Result<(), DomainError> {
Ok(())
}
async fn remove_all_by_actor(&self, _: &str) -> Result<(), DomainError> {
Ok(())
}
async fn get_by_actor_url(
&self,
_: &str,

View File

@@ -711,6 +711,9 @@ impl domain::ports::RemoteGoalRepository for Panic {
async fn remove_by_ap_id(&self, _: &str, _: &str) -> Result<(), DomainError> {
panic!()
}
async fn remove_all_by_actor(&self, _: &str) -> Result<(), DomainError> {
panic!()
}
async fn get_by_actor_url(
&self,
_: &str,