Skip to content

Commit 332fdb5

Browse files
committed
Do not notify users when non-sensitive commands are executed on their behalf
1 parent c1558ce commit 332fdb5

File tree

1 file changed

+27
-9
lines changed

1 file changed

+27
-9
lines changed

src/zulip.rs

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,16 +205,16 @@ async fn handle_command<'a>(
205205
}
206206

207207
let cmd = parse_cli::<ChatCommand, _>(words.into_iter())?;
208-
let output = match cmd {
208+
let output = match &cmd {
209209
ChatCommand::Acknowledge { identifier } => {
210-
acknowledge(&ctx, gh_id, (&identifier).into()).await
210+
acknowledge(&ctx, gh_id, identifier.into()).await
211211
}
212212
ChatCommand::Add { url, description } => {
213213
add_notification(&ctx, gh_id, &url, &description.join(" ")).await
214214
}
215-
ChatCommand::Move { from, to } => move_notification(&ctx, gh_id, from, to).await,
215+
ChatCommand::Move { from, to } => move_notification(&ctx, gh_id, *from, *to).await,
216216
ChatCommand::Meta { index, description } => {
217-
add_meta_notification(&ctx, gh_id, index, &description.join(" ")).await
217+
add_meta_notification(&ctx, gh_id, *index, &description.join(" ")).await
218218
}
219219
ChatCommand::Whoami => whoami_cmd(&ctx, gh_id).await,
220220
ChatCommand::Lookup(cmd) => lookup_cmd(&ctx, cmd).await,
@@ -223,8 +223,8 @@ async fn handle_command<'a>(
223223

224224
let output = output?;
225225

226-
// Let the impersonated person know about the impersonation
227-
if impersonated {
226+
// Let the impersonated person know about the impersonation if the command was sensitive
227+
if impersonated && is_sensitive_command(&cmd) {
228228
let impersonated_zulip_id = to_zulip_id(&ctx.github, gh_id)
229229
.await?
230230
.ok_or_else(|| anyhow::anyhow!("Zulip user for GitHub ID {gh_id} was not found"))?;
@@ -301,12 +301,30 @@ async fn handle_command<'a>(
301301
}
302302
}
303303

304+
/// Returns true if we should notify user who was impersonated by someone who executed this command.
305+
/// More or less, the following holds: `sensitive` == `not read-only`.
306+
fn is_sensitive_command(cmd: &ChatCommand) -> bool {
307+
match cmd {
308+
ChatCommand::Acknowledge { .. }
309+
| ChatCommand::Add { .. }
310+
| ChatCommand::Move { .. }
311+
| ChatCommand::Meta { .. } => true,
312+
ChatCommand::Whoami => false,
313+
ChatCommand::Lookup(_) => false,
314+
ChatCommand::Work(cmd) => match cmd {
315+
WorkqueueCmd::Show => false,
316+
WorkqueueCmd::SetPrLimit { .. } => true,
317+
WorkqueueCmd::SetRotationMode { .. } => true,
318+
},
319+
}
320+
}
321+
304322
/// Commands for working with the workqueue, e.g. showing how many PRs are assigned
305323
/// or modifying the PR review assignment limit.
306324
async fn workqueue_commands(
307325
ctx: &Context,
308326
gh_id: u64,
309-
cmd: WorkqueueCmd,
327+
cmd: &WorkqueueCmd,
310328
) -> anyhow::Result<Option<String>> {
311329
let db_client = ctx.db.get().await;
312330

@@ -364,7 +382,7 @@ async fn workqueue_commands(
364382
WorkqueueCmd::SetPrLimit { limit } => {
365383
let max_assigned_prs = match limit {
366384
WorkqueueLimit::Unlimited => None,
367-
WorkqueueLimit::Limit(limit) => Some(limit),
385+
WorkqueueLimit::Limit(limit) => Some(*limit),
368386
};
369387
upsert_review_prefs(
370388
&db_client,
@@ -457,7 +475,7 @@ async fn whoami_cmd(ctx: &Context, gh_id: u64) -> anyhow::Result<Option<String>>
457475
Ok(Some(output))
458476
}
459477

460-
async fn lookup_cmd(ctx: &Context, cmd: LookupCmd) -> anyhow::Result<Option<String>> {
478+
async fn lookup_cmd(ctx: &Context, cmd: &LookupCmd) -> anyhow::Result<Option<String>> {
461479
let username = match &cmd {
462480
LookupCmd::Zulip { github_username } => github_username.clone(),
463481
// Usernames could contain spaces, so rejoin everything to serve as the username.

0 commit comments

Comments
 (0)