Page MenuHomePhabricator

Phan failing on SecurePoll patches
Closed, DuplicatePublic

Description

This happened a few times today. See https://integration.wikimedia.org/ci/job/mwext-php70-phan-seccheck-docker/21909/console for instance

It seems like parts of code are failing the taint check, but these are NOT parts of code that were modified by that particular patch (in this case https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/SecurePoll/+/468023/ )

Example error from phan:

<error line="63" severity="warning" message="Echoing expression that was not html escaped (Caused by: ./cli/makeSimpleList.php +93)" source="SecurityCheck-XSS"/>

Eyeballing the warnings shows that they are mostly (all?) related to scripts in the ./cli directory. Looking at the code history, it seems like latest phan-related change was ba4bd5734c83 in which @Bawolff made sure SecurePoll passes phan 1.5.0 rules, and indeed, some of the warnings seem to be related to those same parts of code that Brian had "fixed" so to speak.

The issue is I cannot figure out what is exactly wrong here (i.e. how come phan was happy with all other changes between ba4bd5734c83 and today?)

Event Timeline

Huji created this task.Oct 17 2018, 10:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 17 2018, 10:43 PM

I'm semi-away this week. I would suggest moving the check to be non-voting in the mean time

sbassett triaged this task as Medium priority.Oct 15 2019, 7:19 PM