kentonv changed the topic of #sandstorm to: Welcome to #sandstorm: home of all things sandstorm.io. Say hi! | Have a question but no one is here? Try asking in the discussion group: https://groups.google.com/group/sandstorm-dev
wings has joined #sandstorm
<TimMc>
git bisect is fantastic. If you had a window of 1000 commits, it could narrow down on the breaking change in about 10 iterations.
<TimMc>
(In practice, you run into commits that are broken in other ways, so it takes a few more.)
TimMc has quit [Quit: leaving]
TimMc has joined #sandstorm
wings has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
xet7 has quit [Read error: Connection reset by peer]
xet7 has joined #sandstorm
strugee has quit [Ping timeout: 256 seconds]
tian has quit [*.net *.split]
frigginglorious has joined #sandstorm
<kentonv>
JacobWeisz[m], isd, I kind of think we should push a new release reverting the CSP change (by flipping the flag, not reverting the code), and then plan to roll it out again later with a grandfathered list of existing apps
<kentonv>
at least three people have complained and there are probably 10x or more that are affected but just figure they'll stop using sandstorm
<abliss1>
i can see advantages to both sides. it's really nice to flush these out... the latex breakage doesn't sound super bad, since most regular users will probably remember where their favorite buttons are long enough for the admin to apply the workaround... and probably a sharelatex update can be pushed through the market quickly?
<isd>
Yeah, I kindof agree. we should change the default and then ask people to test more extensively (and also fix ShareLaTeX)
<isd>
(agree with kentonv that is)
<abliss1>
every one of these embedded cdn references is a ticking time bomb, and if the upstream host fails or deletes the file, we won't be able to offer a quick workaround then
<JacobWeisz[m]>
I defer to agreeing with Kenton and Ian.
<abliss1>
me too, of course. i just wanted to play devil's advocate for a second.
<kentonv>
abliss1, platforms that randomly break people lose their users. We need to find a way to push this through that doesn't cause disruption.
<JacobWeisz[m]>
That's true but we also thought the breakage of fonts was going to be cosmetic.
<JacobWeisz[m]>
FontAwesome is a painful pox though if used as toolbar buttons.
<isd>
Right, my thinking is we include the option, but disabled in a release, ask people to test stuff, and hope that will shake out any further issues. After a month or so if we don't get any more reports, we flip the default back.
<abliss1>
sounds good. I was just musing whether an intentional controlled breakage now might be worth it to avoid random uncontrolled breakage later.
<JacobWeisz[m]>
I think knowing we broke this and how will help us do better testing as is.
<abliss1>
assuming a flag-flip release isn't too painful, there's little reason not to flip it now at least long enough to fix sharelatex.
<kentonv>
isd, can you send a PR that flips the flag? (I could probably do it but you're less likely to get something wrong.)
<kentonv>
then I'll push a release with it
<JacobWeisz[m]>
And we basically know the list of apps we need to fix to make this go away in the future.
<isd>
will do.
<JacobWeisz[m]>
Should I go ahead and respond as such to the list?
<JacobWeisz[m]>
I deleted the response I was typing to Chris (though I had already sent him the config option).
<isd>
Yeah, I think letting people know what's going on is a good idea.
<kentonv>
I just sent a response
<isd>
we're going to push the option soonish and we should specifically send a call out to the mailing list for people to test.
<isd>
kentonv: opened a pr
<JacobWeisz[m]>
It sounds to me like ShareLatex is popular enough on Sandstorm that we shouldn't let it languish with a 2015 SPK as it is.
<isd>
Agree, we should update that app.
<JacobWeisz[m]>
I opened #3424 to track progress on this project so we have an open issue for it.
<JacobWeisz[m]>
Ian, I was wondering if we should have the install -d flag set the restrictive CSP header though. I guess since that'd be another release away at minimum, it depends how long we expect this runway to be.
<JacobWeisz[m]>
I imagine it would help ensure people are not building new apps or app updates which depend on client side loading scripts.
strugee has joined #sandstorm
<isd>
eh, do we really have enough app updates coming through for that? my feeling is we're not at the scale where that would pay for its implementation cost
<JacobWeisz[m]>
S'why I ask.
<JacobWeisz[m]>
Perhaps not.
<isd>
We should send an email to the list prompting people to turn the config option in on in their dev environments, but I don't think it's worth doing more than that.
<JacobWeisz[m]>
My thought was mostly with regards to vagrant-spk, where people usually don't touch their sandstorm.conf embedded in the VM.
<isd>
I guess you could just add the change to global-setup.sh directly?
<isd>
Probably easier to do it that way; just an unconditional echo 'ALLOW_LEGACY_RELAXED_CSP=false' >> /opt/sandstorm/sandstorm.conf
<JacobWeisz[m]>
That'd be reasonable. And we tell people to use git master for it, so there's no release window concern.
<JacobWeisz[m]>
kentonv: Reminder to push 0.271 commit.
<xet7>
Is there something better than Font Awesome?
<xet7>
I think Wekan embeds it already
<xet7>
that old v4.7
<JacobWeisz[m]>
Yeah, main issue is just when people CDN load crucial parts of their UI.
<JacobWeisz[m]>
Often when literally just declaring a different package will include it directly.
<isd>
Looking at sharelatex... it looks like this package predates vagrant-spk
<JacobWeisz[m]>
Unsurprising. It needed an app update to add the market's metadata fields...
<isd>
There's not any sandstorm-specific build instructions. There's a "sandstorm-port-notes.txt", which tells me david copied /etc/texmf (from whatever system he built this on...?) manually, and then patched a thing...
<isd>
I also haven't looked at the question of database upgrades if we want to actually update it to the latest version.
<isd>
But I'm 95% sure it will be a pain to deal with.
<JacobWeisz[m]>
🤔
<JacobWeisz[m]>
And if we're gonna break the old app, releasing separately isn't ideal.
<JacobWeisz[m]>
Would I be a bad person by suggesting if Sandstorm detects a FontAwesome CDN it should just return FontAwesome from its own static hosting, see also #3223?
<isd>
I'm tempted to suggest: just spk unpack the existing package, shove the fonts in there, find & replace for the urls, pack it back up and release it, then abandon that app id and package a new version of overleaf without legacy
<isd>
IIUC the data is mostly just the latex itself, so moving to a new grain should be easy enough - just copy & paste.
<isd>
We should definitely not have the latest version of the sharelatex package be broken. But I don't think it's unnacceptable to tell users they should switch to this other package if they actually want updates.
<JacobWeisz[m]>
I think people would forgive us for minor upgrading pain if they got 2020 Overleaf?
<isd>
I think we should definitely package the new overleaf. I just question whether it's worth trying to migrate old grains vs. just making it a new app.
<isd>
Like, I don't even know how to build the sharelatex package, it's not documented, it sounds like it literally involves manually copying stuff from a linux distro (whatever david used to package it in 2015) and editing...
<isd>
And the data is some text files? We should pin down migration instructions more carefully, but I kinda want to make sure the old app /works/, but then mark it EOL and say use overleaf if you want updates.
<JacobWeisz[m]>
I think that's adequate.
<JacobWeisz[m]>
Though we need a better way to mark an app EOL too.
<JacobWeisz[m]>
If we're submitting a final update at least we can make it clear in the app's own metadata, I guess.
<isd>
Yeah, put it in the description or whatever...
<isd>
Yeah, so the app has a button to download a .zip with your projects source -- so users wanting a newer version will be expected to do that, then upload it into overleaf.
<JacobWeisz[m]>
Someone should hurry up and go fork Sandstorm if they're superstitious...
<JacobWeisz[m]>
Scary fork number
<isd>
hah
<TimMc>
xet7: I'm pretty sure FontAwesome is to blame for webstes randomly showing weird glyphs instead of icons when my connection is unstable, so I'd say what's better than font awesome is... images.
<TimMc>
(with alt text)
frigginglorious1 has joined #sandstorm
frigginglorious has quit [Ping timeout: 258 seconds]
frigginglorious1 has quit [Read error: Connection reset by peer]
frigginglorious has joined #sandstorm
<xet7>
TimMc: png or svg images?
<JacobWeisz[m]>
TimMc: Personally I blocked enough Google domains that CDN-loaded Material Design icons tend to be what causes that for me now. :P
<JacobWeisz[m]>
But yeah, using custom fonts for icons can do weird things when it doesn't load the external resource.
<JacobWeisz[m]>
#3223 states my opinion that we should serve Sandstorm's own fonts in a manner apps can use them without including them directly. And perhaps we should do the same for FontAwesome if it's common enough.
<TimMc>
xet7: I haven't formed an opinion on that.
<abliss1>
svg images are way better than png for most icons. they compress well and scale to arbitrary resolutions.
<abliss1>
Ian Denhardt i think kenton was saying tat the CSP should have a per-app-id allowlist. if we implement that, then there's no need to mess with updating sharelatex before moving to overleaf with a new appid.
<JacobWeisz[m]>
That's what David said too. Though the idea of leaving a lot of long term holes in the sandbox for compatibility kinda makes me sad.
<JacobWeisz[m]>
We'd want to draw the line between leaving it open for arbitrary nonsense like fonts are slightly different and "the UI doesn't work" level differences.
<TimMc>
Locking it down to just legacy allowlists (for some interim) would mean that you'd "stop the bleed" for new apps.
<JacobWeisz[m]>
I already want to make the change so vagrant-spk uses the strict CSP by default for that purpose.
<JacobWeisz[m]>
But like, I'd rather also not have a ton of 2015 era SPKs in the market too. I see this as a good excuse to update stuff.
<isd>
I don't think we should just whitelist broken packages -- we should fix them.
<isd>
Already working on monkeypatching the sharelatex package.
<isd>
To be clear: right now the sharelatex package hits google's servers (for fonts). If you're using it it is violating basic assumptions of sandstorm's security model.
<isd>
We should not grandfather that in, we should fix it.
<JacobWeisz[m]>
(Contact Otter still calls Google Analytics)
<TimMc>
I guess it boils down to how many apps will be added/modified to rely on external resources by the time you're done rounding up the existing ones.
<TimMc>
If it's a low number, then there's little point in legacy allowlists.
<JacobWeisz[m]>
I am gonna need to be aggressive in reviewing on a strict CSP.
<JacobWeisz[m]>
So we don't get any new ones.
<JacobWeisz[m]>
But we don't get a ton of brand new app submissions.
_whitelogger has joined #sandstorm
frigginglorious has quit [Quit: frigginglorious]
<isd>
Yeah, I think it might make more sense if we were seeing a lot of new submissions.
<isd>
But that's a big part of why I want to tackle all of this stuff _now_; it's only going to get more painful the longer we wait.
<JacobWeisz[m]>
I hadn't been following the ShareLatex repo's issues, but there was a good number of requests there too.
<JacobWeisz[m]>
These old unmaintainable packages are a tech debt too.