Closed Bug 1478898 Opened Last year Closed Last year

ensure bug-id validation works for revisions submitted with arc

Categories

(Conduit :: Phabricator, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: glob, Unassigned)

References

Details

(Keywords: conduit-triaged)

in https://phabricator-dev.allizom.org/D555 i used arc to submit with bug id set to 1.

this should have been rejected at submission time.
See Also: → 1478897
Keywords: conduit-triaged
Priority: -- → P1
This is actually a false alarm. globs account is an admin on the bugzilla-dev server and the code is written that we skip bug id validation if the author is an admin. So it just let him use whatever bug id he chose as long as it was numeric.

But i will still fix the other bug where the feed daemon to handle invalid bug ids properly on its end. I am thinking that if an invalid bug is found by feed daemon, we leave the revision private, and add a comment to the revision stating that the bug id is invalid and let the author then change it themselves. It's really the only thing we can do. We cannot clear the bug id field and make it public and we also would not know which security policy to apply to it. We already do something similar when a bug is private to groups not preset in Phabricator projects.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → INVALID
(In reply to David Lawrence [:dkl] from comment #1)
> This is actually a false alarm. globs account is an admin on the
> bugzilla-dev server and the code is written that we skip bug id validation
> if the author is an admin. So it just let him use whatever bug id he chose
> as long as it was numeric.

why does this override exist?  it sounds like a footgun that should be removed.
Flags: needinfo?(dkl)
(In reply to Byron Jones ‹:glob› 🎈 from comment #2)
> why does this override exist?  it sounds like a footgun that should be
> removed.

https://github.com/mozilla-services/phabricator-extensions/blob/master/extensions/src/differential/customfield/DifferentialBugzillaBugIDValidator.php#L35-L46
Flags: needinfo?(dkl)
(In reply to David Lawrence [:dkl] from comment #3)
> (In reply to Byron Jones ‹:glob› 🎈 from comment #2)
> > why does this override exist?  it sounds like a footgun that should be
> > removed.
> 
> https://github.com/mozilla-services/phabricator-extensions/blob/master/
> extensions/src/differential/customfield/DifferentialBugzillaBugIDValidator.
> php#L35-L46

    // Check to see if the user is an admin; if so, don't validate bug existence
    // because the admin account may not have a BMO account ID associated with it
    // and we don't want to block admins from making revisions private
    // if BMO is down

ah..

> (In reply to David Lawrence [:dkl] from comment #1)
> > This is actually a false alarm. globs account is an admin on the
> > bugzilla-dev server

this override exists for phabricator admins, not bugzilla admins.


to mirror the setup on prod, i've created a glob-admin account on phab-dev and transferred my admin rights to that account.
(In reply to Byron Jones ‹:glob› 🎈 from comment #4)
> > (In reply to David Lawrence [:dkl] from comment #1)
> > > This is actually a false alarm. globs account is an admin on the
> > > bugzilla-dev server
> 
> this override exists for phabricator admins, not bugzilla admins.

Sorry that was a mis-type on my part. I meant phabricator-dev :(
You need to log in before you can comment on or make changes to this bug.