Enable Embedding GitHub Gists as Media in Posts

[emceeaich]

Title:
Enable Embedding GitHub Gists as Media in Posts

Area:
posting, media

Summary:
Enable adding GitHub Gists, which are embedded as scripts, in posts, via the 'add media' option so that DW users can post short blocks of formatted source code for discussion and review.

Description:
Gists are text files hosted on GitHub enabling developers to post short snippets of code for review and comment.

GitHub provides a way to embed a gist on a site, as a script file:

<script src="https://gist.github.com/3290622.js?file=foo.js"></script>

However, DW wisely prevents scripts from being embedded as either media or post content because XSS :)

But adding gist.github.com to a whitelist would enable coders of all skill levels to quickly post nicely formatted snippets of code for review, discussion, and comment.

Poll #11553 Enable Embedding GitHub Gists as Media in Posts

Open to: Registered Users, detailed results viewable to: All, participants: 53

View Respondents

This suggestion:

View Answers

Should be implemented as-is.
13 (24.5%)

Should be implemented with changes. (please comment)
5 (9.4%)

Shouldn't be implemented.
5 (9.4%)

(I have no opinion)
30 (56.6%)

(Other: please comment)
0 (0.0%)

Comments

[ninetydegrees]

If this could never be used for nefarious purposes, then yes.

[kaberett]

+1

[ladyasul]

+1

[emceeaich]

Risks would be:

1) uncaught XSS or CSRF vulnerablity in embedded Gists which would allow attacker to take actions on behalf of the logged in user, or gain access to session cookies.

2) Information leakage, that is GitHub's logs would show a request for the gist embed with xxx.dw.org as referer, allowing an adversary to correlate actions across sites. However, this is the same risk as exposed by embedding media such as YouTube.

[ninetydegrees]

So it's not safer that allowing any other script? If that's the case I don't think we should make an exception.

[emceeaich]

That is not what I said.

What I said, is that if there was a flaw in GitHub's code that propagated a XSS or CSRF, then we would have an exposure.

That is not the same as allowing arbitrary scripts which could be intentionally written with a XSS or CSRF vector.

[swaldman]

Ah, I misunderstood. The Github user can't write the script, the script is simply something generated by github to display the user's text? So that's no worse than being at risk from bugs in other sites such as Youtube?
*changes vote*

[emceeaich]

Yes. Embedding a flash control such as YouTube's means that we're trusting both the flash component and actionscript in the control.

And I would like to have an audit of GitHub's Gist embedding implimentation before proceeding. A review can be a go/no-go part of the process.

[ninetydegrees]

Thanks for the clarification; I'm back to my first comment then. :)

[deborah]

+1

[zvi]

How is this substantially different from including a textbox on an entry?

[erik]

Or using the <code> tag.
If (you know a little HTML)
show (your work)
endif


[emceeaich]

See comment to zvi below, but for substantive blocks of code, formatting, highlighting keywords, and escaping in a CODE element is tedious.

[emceeaich]

GitHub gists provide code formatting and highlighting.

And since DW has moved the open source portion of the project to GitHub, devs would already be using GitHub.

An alternative to Gists would be to look at adding a code formatting library such as http://code.google.com/p/google-code-prettify/ and providing a hook via custom markup.

[swaldman]

I'd be all in favour of adding a tag for pretty code display :-)

[emceeaich]

A code formatting library would achieve most of my aims for this feature. But again, any third party code would need an audit before use.

[zvi]

Oh, I approve of something that would provide code highlighting.

[swaldman]

So, um, would this allow people to run arbitrary scripts embedded in DW pages? Seems like a bad idea in that case, but my web-security-fu is weak so I may be needlessly concerned.

I'd be all for allowing embeds that simply display as text. (or am I being hopelessly naive in making this an easy distinction?)

[emceeaich]

The proposal is limited to allowing embedding of Gists.

It is not intended to allow embedding arbitrary scripts in posts.

Example Gist

[emceeaich]

I've pasted the escaped source from an embed of a GitHub Gist, https://gist.github.com/3290622 below.

The gist itself is the text of a self-executing JavaScript function:

(function foo() {
  var a, b;
  a = b = 1;
})(window);

They are using repeated document.writes to embed the escaped text, and they refer to a stylesheet they write to the document. I'm boggled that they are just adding the link to the CSS instead of finding the document head and adding it there.

document.write('<link rel="stylesheet"
href="https://gist.github.com/stylesheets/gist/embed.css"/>')

document.write('<div id=\"gist-3290622\" class=\"gist\">\n\n      
 <div class=\"gist-file\">\n          <div class=\"gist-data
gist-syntax\">\n              <div
class=\"gist-highlight\"><pre><div class=\'line\'
id=\'LC1\'><span class=\"p\">(<\/span><span
class=\"kd\">function<\/span> <span
class=\"nx\">foo<\/span><span
class=\"p\">()<\/span> <span
class=\"p\">{<\/span><\/div><div class=\'line\'
id=\'LC2\'>  <span class=\"kd\">var<\/span>
<span class=\"nx\">a<\/span><span
class=\"p\">,<\/span> <span
class=\"nx\">b<\/span><span
class=\"p\">;<\/span><\/div><div class=\'line\'
id=\'LC3\'>  <span class=\"nx\">a<\/span>
<span class=\"o\">=<\/span> <span
class=\"nx\">b<\/span> <span class=\"o\">=<\/span>
<span class=\"mi\">1<\/span><span
class=\"p\">;<\/span><\/div><div class=\'line\'
id=\'LC4\'><span class=\"p\">})(<\/span><span
class=\"nb\">window<\/span><span
class=\"p\">);<\/span><\/div><\/pre><\/div>\n
         <\/div>\n\n          <div class=\"gist-meta\">\n   
        <a
href=\"https://gist.github.com/raw/3290622/
05a1d0b7333902a8330802c002c2e129ceff2c48/foo.js\"
style=\"float:right;\">view raw<\/a>\n            <a
href=\"https://gist.github.com/3290622#file_foo.js\"
style=\"float:right;margin-right:10px;color:#666\">foo.js<\/a>\
n            <a href=\"https://gist.github.com/3290622\">This
Gist<\/a> brought to you by <a
href=\"http://github.com\">GitHub<\/a>.\n         
<\/div>\n        <\/div>\n<\/div>\n')

[alexbayleaf]

I voted "should be implemented as-is". At first I had no opinion, but then on reading the comments and remembering that DW development is switching to Github, it made much more sense to have this kind of integration. I wouldn't mind a code markup thingy either, mind you, but gist embedding seems easier.

[kindnesstheorist]

I think "rejectd" should be "rejected" in the tags.