Asciidoctor: support include directives for other asciidoc files

[miltador]

A lot of time has passed since latest comments from devs in #172 and #335.
I think after years things have changed in your infrastructure, why not to bring up this issue again with some more input and news?

For example, AFAIK GitHub started to use containers which could help to isolate things from unintentional access and improve security.

Also take a look at asciidoctor/asciidoctor#1088. The author of Asciidoctor is ready for a conversation about this, there is even proposed a way to implement this with custom include preprocessor. If there are still some concerns about resolving the issue, please provide a constructive feedback so both sides can collaborate.

[chevdor]

I totally 👍 this request. Includes are an amazing feature of Asciidoc over Markdown and would help a lot providing users with good and up-to-date documentation.

I would add image to the same bucket. I think it is not supported today, probably for the same reason includes are not.

[mojavelinux]

I would add image to the same bucket. I think it is not supported today, probably for the same reason includes are not.

The image macros are supported on GitHub. You can see them in action here: https://github.com/asciidoctor/atom-asciidoc-preview

[mojavelinux]

If GitHub is committed to enabling includes for AsciiDoc files, I'm willing to do whatever needs to be done in Asciidoctor to make it happen. I've already added an extension point so the include directive can be handled by a custom function. That allows the GitHub code to resolve the file from the git repository instead of from the file system.

[ericis]

Fully support this. Seems like a basic capability and we're struggling without it 👎
http://asciidoctor.org/docs/user-manual/#include-directive

[nailgilaziev]

it will be great to use a full power of asciidoc from github frontend.
For example I use restdocs & spring to generate snippets for documentation of my API.
Most of documentations is handwritten and only generated snippets was included.
I want to store this documentation in github and collaboratively edit it with others via github instruments(pull requests).

[kavaliro]

This issue was an unpleasant surprise. I agree with @ericis; this is fundamental functionality which is expected by Asciidoc users.

Since @mojavelinux has enabled custom handling , this seems simple:

1. check for infinite recursion
2. only allow relative includes. (i.e., only include files within the repository.)

This seems fairly solvable. Maybe I'm not seeing the full extent of it?

[denuno]

I think it's been a solvable problem for a while now, technically-- at this point it's just someone flipping a couple switches by the looks of it.

This is hands down one of the best features of AsciiDoc and it's a crying shame that there is so little support for it in git-related tooling (GH, Gollum, GitBookIO, etc.).

FWIW, it's not like images are that much safer than other types of content, but for years now I could generate images of my source code and include it that way (ridiculous, as no copy/paste), or generate the content itself and include it that way (which defeats the purpose of nice rendering via GH out of the box).

Someday, it would be nice to move into the brave new future and get some include::sourceFile[tags=tagName] type stuff going... I know not many people like writing docs but this stuff is gold for folks who-- well, may not love writing them, but at least who want to write them as smartly as possible-- and right now those folks have to do the work twice to get it to look nice, basically, and while it can be automated... it shouldn't need to be, in this day and age! 😃

[obilodeau]

So it's pretty clear the reason it hasn't been implemented yet is security. I believe it is also a reason why GitHub's staff doesn't really engage with the community in order to solve that problem because it would require them to expose their threat model and pieces of their architecture which could help attackers.

FWIW, it's not like images are that much safer than other types of content, but for years now I could generate images of my source code and include it that way (ridiculous, as no copy/paste), or generate the content itself and include it that way (which defeats the purpose of nice rendering via GH out of the box).

You are missing the broader point. Include processing is more complex, not because the destination is dangerous, but because of the way it interacts. Just as an example: An include can include includes which could include the first top-level include. This would DoS a naive parser. Analogous to the Billion Laugh Attack and it could be done in so many ways that it's hard to prevent them all with parser restrictions. Another different, yet complex to fix, example: AsciiDoc includes can be URIs, URI includes could be used to perform SSRF attacks in order to explore GitHub's infrastructure in means that shouldn't be allowed.

So I think the reason why GitHub is not doing it is because <1% of their users care and there are numerous ways to abuse the feature so doing it safely is hard. Implementing a hard feature for <1% is not worth the resources (or the risk).

Don't shoot the messenger here: I want AsciiDoc includes to work on GitHub! It's just that I can see why it's a tricky one to implement.

[denuno]

Nothing wrong with sharing how you think-- no shots here, but I disagree that it's security holding this up per se. 😃

Mainly because in the comments above, both injection attacks, and recursion have solutions (and have been solved elsewhere in github infrastructure already)... it's weird that infinite recursion is even on the list, IMHO-- same with the "naive" parser idea... anyone can write a bad parser, we can't protect against that (and I'd be scared if we could!).

Same goes for the security concerns you bring up. They've been brought up, and even addressed, already. If not in this thread/ticket, than in multiple other ones, which leads me to my last refutation, that being the idea this problem affects a negligible number of people.

It seems to be effecting many people (including authors of popular tools who are representing thousands of people), as you can tell by the various tickets around this issue. The same-ish issue (for MD) has the highest number of comments at the moment, and it's just a bump of several other tickets (all of which seem to have "solved" the problems raised but appear to be in a holding pattern). Plus figure a good 85% of people probably don't file a ticket when they hit a problem...

I'll close by saying that security through obscurity isn't really much protection. There's a reason crypto libs have to be published to be valid, and that's because obscurity is "soft" protection at best, and a huge security concern at worst. It's cool GH is sharing the markup plugin-- tho note numbers 2-5 from the docs ("internal code") seem to be a source of bottlenecks.

Pretty sure The Future will be mostly folks who put source code out so it can be verified/checked (been saying this for years, but at least it doesn't sound crazy anymore... how much does OSS power these days?), and if you don't think someone can just call up their friend who works at Large Company X to get a peek at sources... ¯_(ツ)_/¯ anyhow, at this point it's not a tech barrier as far as I can see, and it's fine if something else is holding this up, but there's been little to no feedback from project maintainers, and it's depressing to see so much input and so many orphaned/ghosted PRs going back years and whatnot (in general around this issue). But not that much. 😃 Mainly I'm just tossing in another +1, in a now age-old tradition. 😜

[mojavelinux]

@obilodeau Thank you for sharing your insight on this issue. You provided a lot of context for what the security concern actually entails.

I want to emphasize two things. First, Asciidoctor has from the very beginning offered up the include processor as an extension point (for GitHub, I might add). That means that GitHub could write code that takes over processing of the include, so there's 0 risk of insecure code that they themselves did not introduce. That doesn't make it easy, to be sure, but that should mitigate the third-party risk argument.

Second, I'm willing to make any reasonable change in Asciidoctor that would further accommodate this feature. So far, I have heard absolutely nothing from GitHub on this issue, which doesn't bode well if we expect to make any progress. My offer stands to work with GitHub if they need me to get this feature implemented. (code, documentation, whatever)

Olivier wrote:
An include can include includes which could include the first top-level include.

I don't expect GitHub to use the built-in include processor, but there is protection against this scenario. An include can only go to a fixed number of levels before it's terminated. Most users would be happy with 3-5 levels, if even that.

Olivier wrote:
AsciiDoc includes can be URIs, URI includes could be used to perform SSRF attacks in order to explore GitHub's infrastructure in means that shouldn't be allowed.

I would not except these to work on GitHub. They don't even work on Asciidoctor without a command-line / API flag. I think includes that work within the set of repository files is already a huge boon for us AsciiDoc writers.

Olivier wrote:
Implementing a hard feature for <1% is not worth the resources (or the risk).

If they went with that argument, then I'd say they don't know the AsciiDoc community very well. AsciiDoc users are bonkers for includes, rightfully so. As Denny points out:

It seems to be effecting many people (including authors of popular tools who are representing thousands of people), as you can tell by the various tickets around this issue.

If you want my prediction on this issue, it will be resolved as soon as GitLab implements it. That's not trolling, that's just sound competition in the marketplace. After all, GitLab just implemented stem support (i.e, math expressions) in AsciiDoc files.

[mojavelinux]

Btw, here's a link to corresponding issue in the GitLab issue tracker.

https://gitlab.com/gitlab-org/gitlab-ce/issues/18045

Interesting to note that @jirutka has already submitted a full patch for a custom include processor that works with a repository manager like GitHub and GitLab. So the code is there. We just need the will.

[jyutzler]

I am fine with a very restrictive policy here, even restricting includes to the same repository. My issue is that I have documents that I want modularized but I also want to be able to present them on the web as complete documents and right now I can't do that. I have to kick off a separate process to generate the file then post that. Whether I do this manually or through some sort of agent is irrelevant, it is a nuisance and completely unnecessary.

[kavaliro]

3-5 levels would be perfect imho. Five gives a few levels for document structure and a couple of levels for templates. Three would suffice for most use cases, but five would handle almost all of them.

While I can handle all of the includes before pushing the content to github, effectively that means I'm no longer able to use github as source control (for the same reason our .gitignore files exclude /bin.)

One use case I have uses include to add an svg, and the text fields of the svg are then changed via substitutions. Which really isn't that complex--an svg is just xml after all--but it's not something you can do with the standard image handler.

Count me among those who are bonkers for includes. Includes really are a must have feature for asciidoc.