Skip to content
This repository has been archived by the owner on Feb 7, 2022. It is now read-only.

Prevent side-effects from mirrored markdown syntax #1

Open
gberche-orange opened this issue Jul 26, 2016 · 7 comments
Open

Prevent side-effects from mirrored markdown syntax #1

gberche-orange opened this issue Jul 26, 2016 · 7 comments

Comments

@gberche-orange
Copy link
Member

gberche-orange commented Jul 26, 2016

mirrored markdown content (in story description or comment) may be interpreted by github markdown syntax as:

  • mentions to github users or github organizations
  • github issue cross-references
  • github commit cross references (don't seem to have side effects)
  • disclosing email addresses mentionned in the markdown content, which may lead to increased spam received on these email addresses

This may have the following undesired polluting side effects:

  • notifying people of mirrors
  • adding back cross-reference to mirrors in the mentionned github issues

We need to find a way to avoid such pollution. Some ideas:

  1. escape the markdown content (sacrifying readability) altogether so that no markdown content is interpreted in mirrors
  2. selectively escape markdown syntaxes with side effects
    1. try to escape @mentions such as @gberche with a backslash escape such as /@gberche
@gberche-orange
Copy link
Member Author

gberche-orange commented Jul 26, 2016

More on evaluation of mentions polutions:

Taking the example of https://github.com/gberche-orange/pivotal-tracker-mirror/issues/222 it did not trigger a mention since the CF Gitbot uses the following syntax which is not recognized as a @mention

Filed by [@dcarley](https://github.com/dcarley)

Taking the example of https://github.com/gberche-orange/pivotal-tracker-mirror/issues/224#issuecomment-235375789 the mention did not trigger because the github userid (sreetummidi) and the pivotal tracker ids (stummidi ) were different:

@stummidi As part of this story, we could also address the issue that the entity base url is matched with the assertion consumer service instead of the current url being matched.

For some users with identical ids, mentions polution indeeded triggered, eg in https://github.com/gberche-orange/pivotal-tracker-mirror/issues/87#issuecomment-235332997 for fhanik or madhurab

In general, the @mentions seems indeed notify users even for repos they are not watching. See https://github.com/settings/notifications which mentions " Your notification settings apply to the repositories you’re watching" while https://help.github.com/articles/about-notifications/#types-of-notifications is somewhat more clearly specifying that direct mentions will notify you

https://github.com/gberche-orange/polutting-referencer/issues/2 as an experiment of a poluting mention indeed triggered on non-watched repos.

@gberche-orange
Copy link
Member Author

gberche-orange commented Jul 26, 2016

More on issue cross-reference pollution:

Pollution occur for mirrored stories that were automatically created by the CF git bot for issues or pull-requests

https://github.com/gberche-orange/pivotal-tracker-mirror/issues/220 is using the syntax

[Github Pull Request #398](https://github.com/cloudfoundry/uaa/pull/398)

https://github.com/gberche-orange/pivotal-tracker-mirror/issues/222 is using the syntax:

[Github Issue #21](https://github.com/cloudfoundry/cf-uaa-lib/issues/21)

and additionally the plain text referencing a the cf-uaac#25 issue triggers:

Could these be exposed by `CF::UAA::Scim` so that cloudfoundry/cf-uaac#25 could be implemented?

To fix existing pollution of cross referenced issues, the following alternatives were considered/tested:

  • (tested) disable issues on mirror repo. This transiently removes the cross reference until issues are re-enabled
  • (not-tested) make the mirror repo private (requires $7/months upgrade)
  • (not-tested) delete the mirror repo

@gberche-orange
Copy link
Member Author

More on the email disclosure leading to spam:

In https://github.com/gberche-orange/pivotal-tracker-mirror/issues/223#issuecomment-235375773

Signed-off-by: Madhura Bhave <[email protected]>

The following fixes come to mind:

@gberche-orange
Copy link
Member Author

More on the short-term option of escaping the whole markdown content using and escaping their presence using \:

pros:

  • This seems simple to implement and cover @mentions and cross-references
  • Seems to resist to embedded double quotes ``
  • For most stories and comment, markdown source remains readeable

cons:

@gberche-orange gberche-orange changed the title Exclude side-effects from mirrored markdown syntax Prevent side-effects from mirrored markdown syntax Jul 26, 2016
@gberche-orange
Copy link
Member Author

commit 32ee460 addressed:

I suggested to leave the issue cross-reference as a way to let the CF community discover the mirror issues, and therefore not applying initial suggestions in #1 (comment)

@gberche-orange
Copy link
Member Author

gberche-orange commented Oct 14, 2016

Reopening following @williammartin 's message on cf-dev

The problem is that the mirroring process is enabling implicit back references whereas backlog contributors are not aware of this, and hence could not take care of escaping/sanitizing references they'd like to keep private/without back references.

Proposal to redact all github issue or PR URLs except a whitelist of GH organizations such as "cloudfoundry*" which would be still left as is.

As mentionned into #1 (comment) there are multiple formats to redact (in addition to the officially documented at https://help.github.com/articles/autolinked-references-and-urls/#issues-and-pull-requests ):

https://github.com/jlord/sheetsee.js/issues/27
https://github.com/jlord/sheetsee.js/issues/26
jlord/sheetsee.js#25
[any hyperlink text](https://github.com/jlord/sheetsee.js/issues/24)
[![any image hyperlink text ](https://avatars3.githubusercontent.com/u/4748380?v=3&s=466)](https://github.com/jlord/sheetsee.js/issues/23)
https://github.com/jlord/sheetsee.js/pull/22

which render as the following and indeed trigger cross references:

https://github.com/jlord/sheetsee.js/issues/27
jlord/sheetsee.js#26
jlord/sheetsee.js#25
any hyperlink text
any image hyperlink text
jlord/sheetsee.js#30

Note the following syntax variations may display as urls but don't create cross-refs:

Suggesting to redact them with double backquotes:

``https://github.com/jlord/sheetsee.js/issues/27``
``https://github.com/jlord/sheetsee.js/issues/26``
``jlord/sheetsee.js#25``
``[any hyperlink text](https://github.com/jlord/sheetsee.js/issues/24)``
``[![any image hyperlink text ](https://avatars3.githubusercontent.com/u/4748380?v=3&s=466)](https://github.com/jlord/sheetsee.js/issues/23)``
``https://github.com/jlord/sheetsee.js/pull/30``

which render as and don't trigger cross references:

https://github.com/jlord/sheetsee.js/issues/27
https://github.com/jlord/sheetsee.js/issues/26
jlord/sheetsee.js#25
[any hyperlink text](https://github.com/jlord/sheetsee.js/issues/24)
[![any image hyperlink text ](https://avatars3.githubusercontent.com/u/4748380?v=3&s=466)](https://github.com/jlord/sheetsee.js/issues/23)
https://github.com/jlord/sheetsee.js/pull/30

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@gberche-orange and others