Perhaps https://phan.github.io/demo/ can be used for a start. The demo should do the exact same things, with two differences:
- it should load taint-check
- it should hide all non-taint-check issues.
Perhaps https://phan.github.io/demo/ can be used for a start. The demo should do the exact same things, with two differences:
Tried this on toolforge. It's not straightforward, because:
For future reference, this was achieved by:
Change 643548 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[integration/config@master] Create image for building phan-taint-check-plugin demo
Change 643549 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[integration/config@master] Run new mw-tools-phan-demos-publish job postmerge
Change 643548 merged by jenkins-bot:
[integration/config@master] Create image for building phan-taint-check-plugin demo
Change 643549 merged by jenkins-bot:
[integration/config@master] Run new mw-tools-phan-demos-publish job postmerge
So... https://doc.wikimedia.org/mediawiki-tools-phan-SecurityCheckPlugin/master/demos/ exists! It'll get updated whenever a new commit is merged in phan-taint-check-plugin (takes ~15 min but we can speed it up a lot).
Except it seems to be getting blocked by CSP: Content Security Policy: The page’s settings blocked the loading of a resource at blob:https://doc.wikimedia.org/f8ce60d3-f6e6-4f98-b42b-ab1c401856fc (“script-src”).
Do we need to adjust a CSP rule for this?
Notes on how to speed up the build:
It would be nice to move this into the repo itself since it seems a bit tightly integrated with the manual file list. I *think* we could get away with only keeping copies of our files and then relying on the main phan/demo repo for the C and other stuff?
So this would need to add blob: to script-src (blob: is not included by default with 'self'). You probably also need 'wasm-eval' as well.
Re-opening to track the above. Also, once done, might be good to add an entry to the https://doc.wikimedia.org/ index for easier (re)discovery of the exact URL.
So....I don't think what we're doing here is legal unfortunately. The PHP license is incompatible with the GPL. Normally this isn't a problem because it doesn't affect code that PHP executes, but here we've compiled GPL code with PHP code and distributing it as one wasm binary.
Can we have them as separate wasm binaries so they aren't being distributed together? That would actually be a pretty big win technically because then we're not recompiling PHP on every patch too.
Or if that's not possible and we still want a web demo, we'd need to relicense to Apache 2 or MIT.
Oh, that's a bummer.
but here we've compiled GPL code with PHP code and distributing it as one wasm binary.
I might be missing something but what code is licensed under the GPL here?
Can we have them as separate wasm binaries so they aren't being distributed together? That would actually be a pretty big win technically because then we're not recompiling PHP on every patch too.
Or if that's not possible and we still want a web demo, we'd need to relicense to Apache 2 or MIT.
I've killed the webservice for the time being.
Yuck, I thought I was missing something. Relicensing *might* be an option if everything else fails, but let's think about separating the binaries first. As you said, that would be hugely helpful. I'm not at all expert with emscripten though, so not sure about the feasibility.
@Legoktm I've just tried packaging php and taint-check separately. It should be doable if we:
This will generate a .data and a .js file for taint-check, which should be included in the web page before the main PHP file (docs).
I've just tried this on toolforge. After the usual hour it took to build PHP it errored out due to a stupid syntax error that I left in the build script. So I had to run the final commands manually. In the end the PHP part works, but taint-check doesn't because it cannot be found in the virtual FS. I'm currently trying to understand why.
Would you have some spare time to help me get this to doc.wm.org once I get it to work?
So apparently the JS is not being injected correctly. What emcc does with --preload-file is packaging the file (which we're doing manually) and adding it to --pre-js. Loading the file separately as I tried earlier will not work (perhaps when php.js is loaded it erases everything created by taint-check). But we can still use --pre-js to make it work! This way we end up with:
Would this work? Pushing a PR shortly.
Done. There are also a couple of FIXMEs that I'd like to fix shortly, and I should also rebase my fork to upstream (but index.html was removed so it's not straightforward).
Change 644367 had a related patch set uploaded (by Daimona Eaytoy; author: Legoktm):
[integration/config@master] docker: Switch back to Daimona's repo for phan-taint-check demo
Change 680337 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):
[operations/puppet@production] Relax CSP rules for doc.wm.org/mediawiki-tools-phan-SecurityCheckPlugin
Change 680403 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):
[mediawiki/tools/phan/SecurityCheckPlugin@master] Update demo link
Change 681328 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):
[mediawiki/tools/phan/SecurityCheckPlugin@master] Add scripts for generating a phar archive
Change 681328 merged by jenkins-bot:
[mediawiki/tools/phan/SecurityCheckPlugin@master] Add scripts for generating a phar archive
Change 644367 merged by jenkins-bot:
[integration/config@master] docker: Switch back to Daimona's repo for phan-taint-check demo
Mentioned in SAL (#wikimedia-releng) [2021-04-30T19:21:54Z] <James_F> Docker: Publishing mediawiki-phan-taint-check-demo:0.1.1 for T257301
Change 684019 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):
[integration/config@master] dockerfiles: [mediawiki-phan-taint-check-demo] Require composer-scratch
Change 684019 merged by jenkins-bot:
[integration/config@master] dockerfiles: [mediawiki-phan-taint-check-demo] Provide composer
Change 693969 had a related patch set uploaded (by Jforrester; author: Jforrester):
[integration/config@master] jjb: [mw-tools-phan-demos-publish] Switch to image with composer
Change 693974 had a related patch set uploaded (by Jforrester; author: Jforrester):
[integration/config@master] dockerfiles: [mediawiki-phan-taint-check-demo] Add ext-dom/ext-intl/ext-mbstring
Change 693974 merged by jenkins-bot:
[integration/config@master] dockerfiles: [mediawiki-phan-taint-check-demo] Add ext-dom/ext-intl/ext-mbstring
Change 693990 had a related patch set uploaded (by Jforrester; author: Jforrester):
[integration/config@master] dockerfiles: [mediawiki-phan-taint-check-demo] Install ext-zip and unzip
Change 693990 merged by jenkins-bot:
[integration/config@master] dockerfiles: [mediawiki-phan-taint-check-demo] Install ext-zip and unzip
Change 693969 merged by jenkins-bot:
[integration/config@master] jjb: [mw-tools-phan-demos-publish] Switch to image with composer and PHP exts
OK, this now fails with:
00:07:57.746 emcc:WARNING: --llvm-lto ignored when using llvm backend 00:07:57.870 emcc: warning: EXTRA_EXPORTED_RUNTIME_METHODS is deprecated, please use EXPORTED_RUNTIME_METHODS instead [-Wdeprecated] 00:07:58.004 warning: undefined symbol: getdtablesize (referenced by top-level compiled C/C++ code) 00:07:59.271 warning: undefined symbol: popen (referenced by top-level compiled C/C++ code) 00:07:59.271 + cp out/php.wasm out/php.js out/php.data out/taint-check.js out/taint-check.data .. 00:08:35.956 cp: cannot stat 'out/php.data': No such file or directory 00:08:35.969 Build step 'Execute shell' marked build as failure
This one also happens on toolforge, but seems not to affect the final result (the build script should be updated though).
00:07:57.870 emcc: warning: EXTRA_EXPORTED_RUNTIME_METHODS is deprecated, please use EXPORTED_RUNTIME_METHODS instead [-Wdeprecated]
I think I've never seen this one on toolforge, but this also seems a deprecation warning with no actual effect
00:07:58.004 warning: undefined symbol: getdtablesize (referenced by top-level compiled C/C++ code)
00:07:59.271 warning: undefined symbol: popen (referenced by top-level compiled C/C++ code)
Both seen on toolforge, I haven't investigated why they happen, but again, this shouldn't have any effect; especially because we pass ERROR_ON_UNDEFINED_SYMBOLS=0. (It might be related to the restrictions of running PHP from a web browser).
00:07:59.271 + cp out/php.wasm out/php.js out/php.data out/taint-check.js out/taint-check.data ..
00:08:35.956 cp: cannot stat 'out/php.data': No such file or directory
This is the actual issue. Yet I don't know why we're facing it. None of the issues above should abort the process. Let me check again.
So... It took me quite a while to run everything on toolforge; first of all, it was using emscripten 2.0.8, so I had to upgrade to 2.0.20. Which also required building python from source, since it requires python 3.6+ and monkey-patching it is becoming too hard (and toolforge has 3.5, T268438). I then re-ran the build script and got the same result as CI. Apparently, php.data stopped being created somewhere between emscripten 2.0.8 and 2.0.20. However, the demo still works without it, so I've updated the demo source to copy the file only if it exists (to support older emscripten).
I've just merged r694473 to run the postmerge job, and now the demo works like a charm!
There's still the CSP thing to fix (r680337), but AFAICS that only affects syntax checking for the ace editor. As such, we can already update the demo link IMHO.
Indeed, https://doc.wikimedia.org/mediawiki-tools-phan-SecurityCheckPlugin/master/demos/ but worker blocked by the CSP.
Sorry missed this earlier, but yes. What still needs doing? I'll merge the CSP thing shortly.
We're pretty much done! The demo on doc.wm.org works perfectly now, the CSP thing is only preventing live syntax check (which is not so important). Aside from that, the only remaining thing is to update the demo link in the readme, i.e. r680403.
Change 680403 merged by jenkins-bot:
[mediawiki/tools/phan/SecurityCheckPlugin@master] Update demo link
Change 680337 merged by Dzahn:
[operations/puppet@production] doc: Relax CSP rules for taint-check-demo