Page MenuHomePhabricator

Herald edit log bypasses policy checks
Closed, ResolvedPublic

Description

It seems that Herald Edit Logs bypass policy checks. Specifically, if I go to /herald/history/ then I am able to see the edit history for Herald rules which I otherwise don't have access to see.

For example, at https://secure.phabricator.com/herald/history/ I can see "Rule 104: Created rule 'Watch Support Issues'`, but going to https://secure.phabricator.com/herald/rule/104/ returns "You Shall Not Pass: Restricted Herald Rule".

herald.png (744×1 px, 147 KB)

Revisions and Commits

Event Timeline

joshuaspence raised the priority of this task from to Needs Triage.
joshuaspence updated the task description. (Show Details)
joshuaspence added a project: Herald.
joshuaspence added a subscriber: joshuaspence.

I suppose that the Herald Transcripts have a similar issue.

Yeah, policy rules are a little muddy here. We should probably:

  • Remove the edit log entirely -- it predates transactions, and we've supported transactions for a long time. This mostly just blocked by coming up with a reasonable way to edit the rule diffs (I think we can just dump out some descriptive text blobs) but even without that we'd still be strictly better off by just showing the transactions, since the edit log doesn't have any details anyway.
  • When rendering another user's personal rule in a transcript, maybe don't show the name or conditions, only the effect? This is tricky because we have to balance policies against making it plausible to debug Herald. We could, e.g., show the owner but not the rule name,