{"thread":{"id":"56829","subject":"[PATCH] ci: disallow directional formatting","startedAt":"2021-11-02T12:59:00Z","lastAt":"2021-11-09T13:39:07Z","messageCount":18,"participants":["Johannes Schindelin via GitGitGadget","Ævar Arnfjörð Bjarmason","Taylor Blau","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"440302","messageId":"pull.1071.git.1635857935312.gitgitgadget@gmail.com","threadId":"56829","inReplyTo":null,"subject":"[PATCH] ci: disallow directional formatting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-02T12:58:55Z","receivedAt":"2021-11-02T12:59:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nAs described in https://trojansource.codes/trojan-source.pdf, it is\npossible to abuse directional formatting (a feature of Unicode) to\ndeceive human readers into interpreting code differently from compilers.\n\nIt is highly unlikely that Git's source code wants to contain such\ndirectional formatting in the first place, so let's disallow it.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    ci: disallow directional formatting\n    \n    I just stumbled over\n    https://siliconangle.com/2021/11/01/trojan-source-technique-can-inject-malware-source-code-without-detection/,\n    which details an interesting social-engineering attack: it uses\n    directional formatting in source code to pretend to human readers that\n    the code does something different than it actually does.\n    \n    It is highly unlikely that Git's source code wants to contain such\n    directional formatting in the first place, so let's disallow it.\n    \n    Technically, this is not exactly -rc material, but the paper was just\n    published, and I want us to be safe.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1071%2Fdscho%2Fcheck-for-utf-8-directional-formatting-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1071/dscho/check-for-utf-8-directional-formatting-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1071\n\n .github/workflows/main.yml | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 6ed6a9e8076..7b4b4df03c3 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -289,6 +289,13 @@ jobs:\n     - uses: actions/checkout@v2\n     - run: ci/install-dependencies.sh\n     - run: ci/run-static-analysis.sh\n+    - name: disallow Unicode directional formatting\n+      run: |\n+        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n+        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n+        # could use `git grep -P` with the `\\u` syntax).\n+        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n+          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n   sparse:\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\n\nbase-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n-- \ngitgitgadget\n"},{"id":"440322","messageId":"211102.86tugu1svc.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"pull.1071.git.1635857935312.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-02T15:01:57Z","receivedAt":"2021-11-02T15:26:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 02 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> As described in https://trojansource.codes/trojan-source.pdf, it is\n> possible to abuse directional formatting (a feature of Unicode) to\n> deceive human readers into interpreting code differently from compilers.\n>\n> It is highly unlikely that Git's source code wants to contain such\n> directional formatting in the first place, so let's disallow it.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>     ci: disallow directional formatting\n>     \n>     I just stumbled over\n>     https://siliconangle.com/2021/11/01/trojan-source-technique-can-inject-malware-source-code-without-detection/,\n>     which details an interesting social-engineering attack: it uses\n>     directional formatting in source code to pretend to human readers that\n>     the code does something different than it actually does.\n>     \n>     It is highly unlikely that Git's source code wants to contain such\n>     directional formatting in the first place, so let's disallow it.\n>     \n>     Technically, this is not exactly -rc material, but the paper was just\n>     published, and I want us to be safe.\n\nThere's a parallel discussion about doing something to detect this in\n\"git am\", which for the git project seems like a better place to put\nthis.\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1071%2Fdscho%2Fcheck-for-utf-8-directional-formatting-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1071/dscho/check-for-utf-8-directional-formatting-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1071\n>\n>  .github/workflows/main.yml | 7 +++++++\n>  1 file changed, 7 insertions(+)\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 6ed6a9e8076..7b4b4df03c3 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -289,6 +289,13 @@ jobs:\n>      - uses: actions/checkout@v2\n>      - run: ci/install-dependencies.sh\n>      - run: ci/run-static-analysis.sh\n> +    - name: disallow Unicode directional formatting\n> +      run: |\n> +        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n> +        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n> +        # could use `git grep -P` with the `\\u` syntax).\n> +        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n> +          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n>    sparse:\n>      needs: ci-config\n>      if: needs.ci-config.outputs.enabled == 'yes'\n>\n> base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n\nIt would be easier to maintain this if were added to\nci/run-static-analysis.sh instead, where we have some similar tests, and\nif it lives there we could do away with the double-escaping, then it can\nalso be run manually.\n\nAlso, can't we just pipe \"git ls-files -z\" into \"perl -0ne\" here and use\nits unconditional support for e.g. unicode properties in regexes.\n\nHow will this change impact RTL languages being added to po/?\n"},{"id":"440334","messageId":"YYFdu/+j8ba+hjCF@nand.local","threadId":"56829","inReplyTo":"211102.86tugu1svc.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] ci: disallow directional formatting","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-11-02T15:48:11Z","receivedAt":"2021-11-02T15:49:50Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 02, 2021 at 04:01:57PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> There's a parallel discussion about doing something to detect this in\n> \"git am\", which for the git project seems like a better place to put\n> this.\n\nI don't think that one impacts the other necessarily. Having `git am`\nguard against this would probably be sufficient to protect Junio\naccidentally apply something containing directional formatting to his\ntree unknowingly.\n\nBut the idea that we rely on the import mechanism to protect against\nthis doesn't sit well with me. Ultimately, we should be relying on a\nstatic check like below to ensure that directional formatting hasn't\nentered the tree by any mechanism (not just 'git am').\n\n> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> > index 6ed6a9e8076..7b4b4df03c3 100644\n> > --- a/.github/workflows/main.yml\n> > +++ b/.github/workflows/main.yml\n> > @@ -289,6 +289,13 @@ jobs:\n> >      - uses: actions/checkout@v2\n> >      - run: ci/install-dependencies.sh\n> >      - run: ci/run-static-analysis.sh\n> > +    - name: disallow Unicode directional formatting\n> > +      run: |\n> > +        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n> > +        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n> > +        # could use `git grep -P` with the `\\u` syntax).\n> > +        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n> > +          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n> >    sparse:\n> >      needs: ci-config\n> >      if: needs.ci-config.outputs.enabled == 'yes'\n> >\n> > base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n>\n> It would be easier to maintain this if were added to\n> ci/run-static-analysis.sh instead, where we have some similar tests, and\n> if it lives there we could do away with the double-escaping, then it can\n> also be run manually.\n>\n> Also, can't we just pipe \"git ls-files -z\" into \"perl -0ne\" here and use\n> its unconditional support for e.g. unicode properties in regexes.\n\nI agree that the double-escaping is ugly. I think that this would be\neasier to maintain if it lived in ci/run-static-analysis.sh or its own\nscript like ci/check-directional-formatting.sh.\n\nAnd yes, constructing a byte pattern is a little odd as well, but I\nthink that it's the best you can do if you're limited to running 'git\ngrep' without libpcre. I wondered if we could depend on perl being\naround during CI, but as far as I know we can since install Perl modules\nin ci/install-dependencies.sh and use Perl extensively through the test\nsuite.\n\nThanks,\nTaylor\n"},{"id":"440335","messageId":"211102.86lf261q2e.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"YYFdu/+j8ba+hjCF@nand.local","subject":"Re: [PATCH] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-02T16:03:22Z","receivedAt":"2021-11-02T16:36:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 02 2021, Taylor Blau wrote:\n\n> On Tue, Nov 02, 2021 at 04:01:57PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> There's a parallel discussion about doing something to detect this in\n>> \"git am\", which for the git project seems like a better place to put\n>> this.\n>\n> I don't think that one impacts the other necessarily. Having `git am`\n> guard against this would probably be sufficient to protect Junio\n> accidentally apply something containing directional formatting to his\n> tree unknowingly.\n>\n> But the idea that we rely on the import mechanism to protect against\n> this doesn't sit well with me. Ultimately, we should be relying on a\n> static check like below to ensure that directional formatting hasn't\n> entered the tree by any mechanism (not just 'git am').\n>\n>> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n>> > index 6ed6a9e8076..7b4b4df03c3 100644\n>> > --- a/.github/workflows/main.yml\n>> > +++ b/.github/workflows/main.yml\n>> > @@ -289,6 +289,13 @@ jobs:\n>> >      - uses: actions/checkout@v2\n>> >      - run: ci/install-dependencies.sh\n>> >      - run: ci/run-static-analysis.sh\n>> > +    - name: disallow Unicode directional formatting\n>> > +      run: |\n>> > +        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n>> > +        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n>> > +        # could use `git grep -P` with the `\\u` syntax).\n>> > +        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n>> > +          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n>> >    sparse:\n>> >      needs: ci-config\n>> >      if: needs.ci-config.outputs.enabled == 'yes'\n>> >\n>> > base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n>>\n>> It would be easier to maintain this if were added to\n>> ci/run-static-analysis.sh instead, where we have some similar tests, and\n>> if it lives there we could do away with the double-escaping, then it can\n>> also be run manually.\n>>\n>> Also, can't we just pipe \"git ls-files -z\" into \"perl -0ne\" here and use\n>> its unconditional support for e.g. unicode properties in regexes.\n>\n> I agree that the double-escaping is ugly. I think that this would be\n> easier to maintain if it lived in ci/run-static-analysis.sh or its own\n> script like ci/check-directional-formatting.sh.\n>\n> And yes, constructing a byte pattern is a little odd as well, but I\n> think that it's the best you can do if you're limited to running 'git\n> grep' without libpcre. I wondered if we could depend on perl being\n> around during CI, but as far as I know we can since install Perl modules\n> in ci/install-dependencies.sh and use Perl extensively through the test\n> suite.\n\nWe can hard depend on anything listed in [1][2], in this case there's a\nPerl 5.30 available.\n\nWe don't need any Perl modules, it's all Perl-native regex features and\na small for-loop.\n\nOn the topic at large I wonder how much we need to worry about this at\nall, seems like somewher between case of \"the anglosphere discovers\nscary foreign characters in Unicode again\" and \"security researcher\ncreates scary landing page for well-known Unicode edge-case\"[3] :)\n\nIn this particular case the test cases involved seem extremely contrived\nand unlikely to be something we'd end up with in our code in any case,\neven in a case where all the reviewers would be fooled by this method in\nitself.\n\nI.e. since you can't use these to \"fold\" lines in the eyes of a human\nviewer it's all rather contrived cases where comments and valid C code\nlives on the same line, e.g.:\n\n    /*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n        printf(\"You are an admin.\\n\");\n    /* end admins only ‮ { ⁦*/\n\nWhich at least in Emacs is highlighted the right way, which is the first\nmajor clue, and having tested some of these just now on AIX I've got xlc\nwhining about some cases that gcc/clang will happily pass by, so our\ntooling at large will probably catch these anyway...\n\n1. https://github.com/actions/virtual-environments\n2. https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md\n3. https://www.trojansource.codes/\n"},{"id":"440337","messageId":"211102.86h7cu1p7b.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"nycvar.QRO.7.76.6.2111021706230.56@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-02T16:38:07Z","receivedAt":"2021-11-02T16:46:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 02 2021, Johannes Schindelin wrote:\n\n> Hi Taylor,\n>\n> On Tue, 2 Nov 2021, Taylor Blau wrote:\n>\n>> On Tue, Nov 02, 2021 at 04:01:57PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> > There's a parallel discussion about doing something to detect this in\n>> > \"git am\", which for the git project seems like a better place to put\n>> > this.\n>>\n>> I don't think that one impacts the other necessarily. Having `git am`\n>> guard against this would probably be sufficient to protect Junio\n>> accidentally apply something containing directional formatting to his\n>> tree unknowingly.\n>>\n>> But the idea that we rely on the import mechanism to protect against\n>> this doesn't sit well with me. Ultimately, we should be relying on a\n>> static check like below to ensure that directional formatting hasn't\n>> entered the tree by any mechanism (not just 'git am').\n>\n> Yep, the `git am` change and the CI change are addressing two very\n> different concerns.\n>\n>> > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n>> > > index 6ed6a9e8076..7b4b4df03c3 100644\n>> > > --- a/.github/workflows/main.yml\n>> > > +++ b/.github/workflows/main.yml\n>> > > @@ -289,6 +289,13 @@ jobs:\n>> > >      - uses: actions/checkout@v2\n>> > >      - run: ci/install-dependencies.sh\n>> > >      - run: ci/run-static-analysis.sh\n>> > > +    - name: disallow Unicode directional formatting\n>> > > +      run: |\n>> > > +        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n>> > > +        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n>> > > +        # could use `git grep -P` with the `\\u` syntax).\n>> > > +        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n>> > > +          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n>> > >    sparse:\n>> > >      needs: ci-config\n>> > >      if: needs.ci-config.outputs.enabled == 'yes'\n>> > >\n>> > > base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n>> >\n>> > It would be easier to maintain this if were added to\n>> > ci/run-static-analysis.sh instead, where we have some similar tests, and\n>> > if it lives there we could do away with the double-escaping, then it can\n>> > also be run manually.\n>> >\n>> > Also, can't we just pipe \"git ls-files -z\" into \"perl -0ne\" here and use\n>> > its unconditional support for e.g. unicode properties in regexes.\n>>\n>> I agree that the double-escaping is ugly. I think that this would be\n>> easier to maintain if it lived in ci/run-static-analysis.sh or its own\n>> script like ci/check-directional-formatting.sh.\n>\n> That's a good idea, I will put it into its own script in ci/.\n>\n>> And yes, constructing a byte pattern is a little odd as well, but I\n>> think that it's the best you can do if you're limited to running 'git\n>> grep' without libpcre. I wondered if we could depend on perl being\n>> around during CI, but as far as I know we can since install Perl modules\n>> in ci/install-dependencies.sh and use Perl extensively through the test\n>> suite.\n>\n> Perl alone won't fix anything. A crucial part of the `git grep` invocation\n> is the `-I` option: ignore binary files.\n\nAs noted in [1] I'm rather skeptical of us needing this at all, but why\nis \"-I\" needed over asking \"git grep\" or \"git ls-files\" to exclude\nbinary files?\n\n    git ls-files ':!(attr:binary)'\n\nI.e. why do ad-hoc binary detection on the fly in git.git when we should\nalready be marking what files are binary?\n\nIf this check shows a false positive due to a binary file isn't that a\ngood thing (sans LTR issues I mentioned upthread), i.e. we should be\nadding that file to .gitattributes, no?\n\nIn any case, I meant that the match on the RHS might be easier with\nPerl, in such a pipeline you could always farm out the binary detection\nto GNU grep on the LHS or whatever. Maybe you don't want to do it with\nPerl, but using the -I option seems like a bad idea in either case.\n\n1. https://lore.kernel.org/git/211102.86lf261q2e.gmgdl@evledraar.gmail.com/\n"},{"id":"440338","messageId":"nycvar.QRO.7.76.6.2111021706230.56@tvgsbejvaqbjf.bet","threadId":"56829","inReplyTo":"YYFdu/+j8ba+hjCF@nand.local","subject":"Re: [PATCH] ci: disallow directional formatting","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-02T16:12:32Z","receivedAt":"2021-11-02T16:47:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Taylor,\n\nOn Tue, 2 Nov 2021, Taylor Blau wrote:\n\n> On Tue, Nov 02, 2021 at 04:01:57PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> > There's a parallel discussion about doing something to detect this in\n> > \"git am\", which for the git project seems like a better place to put\n> > this.\n>\n> I don't think that one impacts the other necessarily. Having `git am`\n> guard against this would probably be sufficient to protect Junio\n> accidentally apply something containing directional formatting to his\n> tree unknowingly.\n>\n> But the idea that we rely on the import mechanism to protect against\n> this doesn't sit well with me. Ultimately, we should be relying on a\n> static check like below to ensure that directional formatting hasn't\n> entered the tree by any mechanism (not just 'git am').\n\nYep, the `git am` change and the CI change are addressing two very\ndifferent concerns.\n\n> > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> > > index 6ed6a9e8076..7b4b4df03c3 100644\n> > > --- a/.github/workflows/main.yml\n> > > +++ b/.github/workflows/main.yml\n> > > @@ -289,6 +289,13 @@ jobs:\n> > >      - uses: actions/checkout@v2\n> > >      - run: ci/install-dependencies.sh\n> > >      - run: ci/run-static-analysis.sh\n> > > +    - name: disallow Unicode directional formatting\n> > > +      run: |\n> > > +        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n> > > +        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n> > > +        # could use `git grep -P` with the `\\u` syntax).\n> > > +        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n> > > +          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n> > >    sparse:\n> > >      needs: ci-config\n> > >      if: needs.ci-config.outputs.enabled == 'yes'\n> > >\n> > > base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n> >\n> > It would be easier to maintain this if were added to\n> > ci/run-static-analysis.sh instead, where we have some similar tests, and\n> > if it lives there we could do away with the double-escaping, then it can\n> > also be run manually.\n> >\n> > Also, can't we just pipe \"git ls-files -z\" into \"perl -0ne\" here and use\n> > its unconditional support for e.g. unicode properties in regexes.\n>\n> I agree that the double-escaping is ugly. I think that this would be\n> easier to maintain if it lived in ci/run-static-analysis.sh or its own\n> script like ci/check-directional-formatting.sh.\n\nThat's a good idea, I will put it into its own script in ci/.\n\n> And yes, constructing a byte pattern is a little odd as well, but I\n> think that it's the best you can do if you're limited to running 'git\n> grep' without libpcre. I wondered if we could depend on perl being\n> around during CI, but as far as I know we can since install Perl modules\n> in ci/install-dependencies.sh and use Perl extensively through the test\n> suite.\n\nPerl alone won't fix anything. A crucial part of the `git grep` invocation\nis the `-I` option: ignore binary files.\n\nCiao,\nDscho\n"},{"id":"440373","messageId":"pull.1071.v2.git.1635942236065.gitgitgadget@gmail.com","threadId":"56829","inReplyTo":"pull.1071.git.1635857935312.gitgitgadget@gmail.com","subject":"[PATCH v2] ci: disallow directional formatting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-03T12:23:55Z","receivedAt":"2021-11-03T12:24:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nAs described in https://trojansource.codes/trojan-source.pdf, it is\npossible to abuse directional formatting (a feature of Unicode) to\ndeceive human readers into interpreting code differently from compilers.\n\nFor example, an \"if ()\" expression could be enclosed in a comment, but\nrendered as if it was outside of that comment. In effect, this could\nfool a reviewer into misinterpreting the code flow as benign when it is\nnot.\n\nIt is highly unlikely that Git's source code wants to contain such\ndirectional formatting in the first place, so let's just disallow it.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    ci: disallow directional formatting\n    \n    I just stumbled over\n    https://siliconangle.com/2021/11/01/trojan-source-technique-can-inject-malware-source-code-without-detection/,\n    which details an interesting social-engineering attack: it uses\n    directional formatting in source code to pretend to human readers that\n    the code does something different than it actually does.\n    \n    It is highly unlikely that Git's source code wants to contain such\n    directional formatting in the first place, so let's disallow it.\n    \n    Technically, this is not exactly -rc material, but the paper was just\n    published, and I want us to be safe.\n    \n    Changes since v1:\n    \n     * The code was moved into a script in ci/.\n     * We use git ls-files now to exclude files marked as binary in the Git\n       attributes.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1071%2Fdscho%2Fcheck-for-utf-8-directional-formatting-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1071/dscho/check-for-utf-8-directional-formatting-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1071\n\nRange-diff vs v1:\n\n 1:  6a1661fd887 ! 1:  bbf963695ba ci: disallow directional formatting\n     @@ Commit message\n          possible to abuse directional formatting (a feature of Unicode) to\n          deceive human readers into interpreting code differently from compilers.\n      \n     +    For example, an \"if ()\" expression could be enclosed in a comment, but\n     +    rendered as if it was outside of that comment. In effect, this could\n     +    fool a reviewer into misinterpreting the code flow as benign when it is\n     +    not.\n     +\n          It is highly unlikely that Git's source code wants to contain such\n     -    directional formatting in the first place, so let's disallow it.\n     +    directional formatting in the first place, so let's just disallow it.\n      \n          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n      \n     @@ .github/workflows/main.yml: jobs:\n           - uses: actions/checkout@v2\n           - run: ci/install-dependencies.sh\n           - run: ci/run-static-analysis.sh\n     -+    - name: disallow Unicode directional formatting\n     -+      run: |\n     -+        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n     -+        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n     -+        # could use `git grep -P` with the `\\u` syntax).\n     -+        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n     -+          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n     ++    - run: ci/check-directional-formatting.sh\n         sparse:\n           needs: ci-config\n           if: needs.ci-config.outputs.enabled == 'yes'\n     +\n     + ## ci/check-directional-formatting.sh (new) ##\n     +@@\n     ++#!/bin/bash\n     ++\n     ++# This script verifies that the non-binary files tracked in the Git index do\n     ++# not contain any Unicode directional formatting: such formatting could be used\n     ++# to deceive reviewers into interpreting code differently from the compiler.\n     ++# This is intended to run on an Ubuntu agent in a GitHub workflow.\n     ++#\n     ++# `git grep` as well as GNU grep do not handle `\\u` as a way to specify UTF-8.\n     ++# A PCRE-enabled `git grep` would handle `\\u` as desired, but Ubuntu does\n     ++# not build its `git` packages with PCRE support.\n     ++#\n     ++# To work around that, we use `printf` to produce the pattern as a byte\n     ++# sequence, and then feed that to `git grep` as a byte sequence (setting\n     ++# `LC_CTYPE` to make sure that the arguments are interpreted as intended).\n     ++#\n     ++# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n     ++# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n     ++# for example, would use a `printf` that does not understand that syntax.\n     ++\n     ++# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n     ++# U+2066..U+2069: LRI, RLI, FSI and PDI\n     ++regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n     ++\n     ++! git ls-files -z ':(attr:!binary)' |\n     ++LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n\n\n .github/workflows/main.yml         |  1 +\n ci/check-directional-formatting.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 26 insertions(+)\n create mode 100755 ci/check-directional-formatting.sh\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 6ed6a9e8076..36b7a0bee38 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -289,6 +289,7 @@ jobs:\n     - uses: actions/checkout@v2\n     - run: ci/install-dependencies.sh\n     - run: ci/run-static-analysis.sh\n+    - run: ci/check-directional-formatting.sh\n   sparse:\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\ndiff --git a/ci/check-directional-formatting.sh b/ci/check-directional-formatting.sh\nnew file mode 100755\nindex 00000000000..ab894715cf1\n--- /dev/null\n+++ b/ci/check-directional-formatting.sh\n@@ -0,0 +1,25 @@\n+#!/bin/bash\n+\n+# This script verifies that the non-binary files tracked in the Git index do\n+# not contain any Unicode directional formatting: such formatting could be used\n+# to deceive reviewers into interpreting code differently from the compiler.\n+# This is intended to run on an Ubuntu agent in a GitHub workflow.\n+#\n+# `git grep` as well as GNU grep do not handle `\\u` as a way to specify UTF-8.\n+# A PCRE-enabled `git grep` would handle `\\u` as desired, but Ubuntu does\n+# not build its `git` packages with PCRE support.\n+#\n+# To work around that, we use `printf` to produce the pattern as a byte\n+# sequence, and then feed that to `git grep` as a byte sequence (setting\n+# `LC_CTYPE` to make sure that the arguments are interpreted as intended).\n+#\n+# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n+# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n+# for example, would use a `printf` that does not understand that syntax.\n+\n+# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n+# U+2066..U+2069: LRI, RLI, FSI and PDI\n+regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n+\n+! git ls-files -z ':(attr:!binary)' |\n+LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n\nbase-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n-- \ngitgitgadget\n"},{"id":"440386","messageId":"xmqq35od88bh.fsf@gitster.g","threadId":"56829","inReplyTo":"YYFdu/+j8ba+hjCF@nand.local","subject":"Re: [PATCH] ci: disallow directional formatting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-03T17:20:34Z","receivedAt":"2021-11-03T17:20:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Tue, Nov 02, 2021 at 04:01:57PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> There's a parallel discussion about doing something to detect this in\n>> \"git am\", which for the git project seems like a better place to put\n>> this.\n>\n> I don't think that one impacts the other necessarily. Having `git am`\n> guard against this would probably be sufficient to protect Junio\n> accidentally apply something containing directional formatting to his\n> tree unknowingly.\n>\n> But the idea that we rely on the import mechanism to protect against\n> this doesn't sit well with me. Ultimately, we should be relying on a\n> static check like below to ensure that directional formatting hasn't\n> entered the tree by any mechanism (not just 'git am').\n\nYes.  Quite honestly, such a check shouldn't be in \"am\" proper at\nall.\n\nRather, for am users who care, they should protect themselves with\nsomething like the pre-applypatch hook, which can perform the same\ncheck as their pre-commit hook to protect their other commits.\n"},{"id":"440391","messageId":"xmqqfssd6rwu.fsf@gitster.g","threadId":"56829","inReplyTo":"pull.1071.v2.git.1635942236065.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] ci: disallow directional formatting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-03T18:00:17Z","receivedAt":"2021-11-03T18:00:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> +# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n> +# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n> +# for example, would use a `printf` that does not understand that syntax.\n\nWow, the situation is ugly and the solution is understandable.\n\nShouldn't we call this script *.bash instead of *.sh then, though?\n\nOther than that, looks good to me.  Thanks.\n\n> +\n> +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n> +# U+2066..U+2069: LRI, RLI, FSI and PDI\n> +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n> +\n> +! git ls-files -z ':(attr:!binary)' |\n> +LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n>\n> base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n"},{"id":"440392","messageId":"211103.86zgqlhzvz.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"pull.1071.v2.git.1635942236065.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-03T16:36:12Z","receivedAt":"2021-11-03T18:12:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 03 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> As described in https://trojansource.codes/trojan-source.pdf, it is\n> possible to abuse directional formatting (a feature of Unicode) to\n> deceive human readers into interpreting code differently from compilers.\n>\n> For example, an \"if ()\" expression could be enclosed in a comment, but\n> rendered as if it was outside of that comment. In effect, this could\n> fool a reviewer into misinterpreting the code flow as benign when it is\n> not.\n>\n> It is highly unlikely that Git's source code wants to contain such\n> directional formatting in the first place, so let's just disallow it.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>     ci: disallow directional formatting\n>     \n>     I just stumbled over\n>     https://siliconangle.com/2021/11/01/trojan-source-technique-can-inject-malware-source-code-without-detection/,\n>     which details an interesting social-engineering attack: it uses\n>     directional formatting in source code to pretend to human readers that\n>     the code does something different than it actually does.\n>     \n>     It is highly unlikely that Git's source code wants to contain such\n>     directional formatting in the first place, so let's disallow it.\n>     \n>     Technically, this is not exactly -rc material, but the paper was just\n>     published, and I want us to be safe.\n>     \n>     Changes since v1:\n>     \n>      * The code was moved into a script in ci/.\n>      * We use git ls-files now to exclude files marked as binary in the Git\n>        attributes.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1071%2Fdscho%2Fcheck-for-utf-8-directional-formatting-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1071/dscho/check-for-utf-8-directional-formatting-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1071\n>\n> Range-diff vs v1:\n>\n>  1:  6a1661fd887 ! 1:  bbf963695ba ci: disallow directional formatting\n>      @@ Commit message\n>           possible to abuse directional formatting (a feature of Unicode) to\n>           deceive human readers into interpreting code differently from compilers.\n>       \n>      +    For example, an \"if ()\" expression could be enclosed in a comment, but\n>      +    rendered as if it was outside of that comment. In effect, this could\n>      +    fool a reviewer into misinterpreting the code flow as benign when it is\n>      +    not.\n>      +\n>           It is highly unlikely that Git's source code wants to contain such\n>      -    directional formatting in the first place, so let's disallow it.\n>      +    directional formatting in the first place, so let's just disallow it.\n>       \n>           Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>       \n>      @@ .github/workflows/main.yml: jobs:\n>            - uses: actions/checkout@v2\n>            - run: ci/install-dependencies.sh\n>            - run: ci/run-static-analysis.sh\n>      -+    - name: disallow Unicode directional formatting\n>      -+      run: |\n>      -+        # Use UTF-8-aware `printf` to feed a byte pattern to non-UTF-8-aware `git grep`\n>      -+        # (Ubuntu's `git grep` is compiled without support for libpcre, otherwise we\n>      -+        # could use `git grep -P` with the `\\u` syntax).\n>      -+        ! LANG=C git grep -Il \"$(LANG=C.UTF-8 printf \\\n>      -+          '\\\\(\\u202a\\\\|\\u202b\\\\|\\u202c\\\\|\\u202d\\\\|\\u202e\\\\|\\u2066\\\\|\\u2067\\\\|\\u2068\\\\|\\u2069\\\\)')\"\n>      ++    - run: ci/check-directional-formatting.sh\n>          sparse:\n>            needs: ci-config\n>            if: needs.ci-config.outputs.enabled == 'yes'\n>      +\n>      + ## ci/check-directional-formatting.sh (new) ##\n>      +@@\n>      ++#!/bin/bash\n>      ++\n>      ++# This script verifies that the non-binary files tracked in the Git index do\n>      ++# not contain any Unicode directional formatting: such formatting could be used\n>      ++# to deceive reviewers into interpreting code differently from the compiler.\n>      ++# This is intended to run on an Ubuntu agent in a GitHub workflow.\n>      ++#\n>      ++# `git grep` as well as GNU grep do not handle `\\u` as a way to specify UTF-8.\n>      ++# A PCRE-enabled `git grep` would handle `\\u` as desired, but Ubuntu does\n>      ++# not build its `git` packages with PCRE support.\n>      ++#\n>      ++# To work around that, we use `printf` to produce the pattern as a byte\n>      ++# sequence, and then feed that to `git grep` as a byte sequence (setting\n>      ++# `LC_CTYPE` to make sure that the arguments are interpreted as intended).\n>      ++#\n>      ++# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n>      ++# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n>      ++# for example, would use a `printf` that does not understand that syntax.\n>      ++\n>      ++# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n>      ++# U+2066..U+2069: LRI, RLI, FSI and PDI\n>      ++regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n>      ++\n>      ++! git ls-files -z ':(attr:!binary)' |\n>      ++LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n>\n>\n>  .github/workflows/main.yml         |  1 +\n>  ci/check-directional-formatting.sh | 25 +++++++++++++++++++++++++\n>  2 files changed, 26 insertions(+)\n>  create mode 100755 ci/check-directional-formatting.sh\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 6ed6a9e8076..36b7a0bee38 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -289,6 +289,7 @@ jobs:\n>      - uses: actions/checkout@v2\n>      - run: ci/install-dependencies.sh\n>      - run: ci/run-static-analysis.sh\n> +    - run: ci/check-directional-formatting.sh\n>    sparse:\n>      needs: ci-config\n>      if: needs.ci-config.outputs.enabled == 'yes'\n> diff --git a/ci/check-directional-formatting.sh b/ci/check-directional-formatting.sh\n> new file mode 100755\n> index 00000000000..ab894715cf1\n> --- /dev/null\n> +++ b/ci/check-directional-formatting.sh\n> @@ -0,0 +1,25 @@\n> +#!/bin/bash\n> +\n> +# This script verifies that the non-binary files tracked in the Git index do\n> +# not contain any Unicode directional formatting: such formatting could be used\n> +# to deceive reviewers into interpreting code differently from the compiler.\n> +# This is intended to run on an Ubuntu agent in a GitHub workflow.\n> +#\n> +# `git grep` as well as GNU grep do not handle `\\u` as a way to specify UTF-8.\n> +# A PCRE-enabled `git grep` would handle `\\u` as desired, but Ubuntu does\n> +# not build its `git` packages with PCRE support.\n\nI just took you at your word before, but, no. It does have PCRE. It's\nthis Ubuntu version:\nhttps://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md\ni.e. \"Ubuntu 20.04.3 LTS\".\n\nDebian's \"git\" package builds with libpcre, and Ubuntu's page about it\nsuggests it does the same: https://packages.ubuntu.com/focal/git\n\nAnd if you try to push e.g. a grep for \"git grep -P 'foo(?=bar)' you'll\nfind that it works.\n\nWhat we haven't done in \"git grep\" is to compile the pattern with\nPCRE2_ALT_BSUX, which is probably what you ran into. I.e. it doesn't\ngrok \\u by default, we should probably support that, but you don't need\nit.\n\n> +#\n> +# To work around that, we use `printf` to produce the pattern as a byte\n> +# sequence, and then feed that to `git grep` as a byte sequence (setting\n> +# `LC_CTYPE` to make sure that the arguments are interpreted as intended).\n> +#\n> +# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n> +# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n> +# for example, would use a `printf` that does not understand that syntax.\n> +\n> +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n> +# U+2066..U+2069: LRI, RLI, FSI and PDI\n> +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n> +\n> +! git ls-files -z ':(attr:!binary)' |\n> +LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n\nFWIW with that ls-files suggestion of mine I meant that would make sense\nas a way to pipe it into Perl since doing this with a Unicode-aware\nregex engine is less painful. I.e. you'd be able to just name the\nUnicode range.\n\nBut since you didn't go for that surely this whole ls-files -> xargs ->\ngit grep pipeline isn't needed, and you'd just want:\n\n    ! git -P grep -nE \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" ':(attr:!binary)'\n\nI added the -n for readability of the output, a matter of taste, and -P\nto \"git\" for ease of ad-hoc testing. A test of that on the researcher's\ntrojan-source.git repo:\n    \n    $ ! git -P grep -nE \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" ':(attr:!binary)'; echo $?\n    C#/commenting-out.csx:4:/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n    C#/commenting-out.csx:6:/* end admins only ‮ { ⁦*/\n    C#/stretched-string.csx:4:if (access_level != \"user‮ ⁦// Check if admin⁩ ⁦\") {\n    C++/commenting-out.cpp:5:    /*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n    C++/commenting-out.cpp:7:    /* end admins only ‮ { ⁦*/\n    C++/stretched-string.cpp:6:    if (access_level.compare(\"user‮ ⁦// Check if admin⁩ ⁦\")) {\n    C/commenting-out.c:6:    /*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n    C/commenting-out.c:8:    /* end admins only ‮ { ⁦*/\n    C/early-return.c:4:    /* Say hello; newline⁧ /*/ return 0 ;\n    C/stretched-string.c:6:    if (strcmp(access_level, \"user‮ ⁦// Check if admin⁩ ⁦\")) {\n    Go/commenting-out.go:9:    /*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n    Go/commenting-out.go:11:    /* end admins only ‮ { ⁦*/\n    Go/stretched-string.go:7:       if accessLevel != \"user‮ ⁦// Check if admin⁩ ⁦\" {\n    Java/CommentingOut.java:5:        /*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n    Java/CommentingOut.java:7:        /* end admins only ‮ { ⁦*/\n    Java/StretchedString.java:5:        if (accessLevel != \"user‮ ⁦// Check if admin⁩ ⁦\") {\n    JavaScript/commenting-out.js:4:/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */\n    JavaScript/commenting-out.js:6:/* end admins only ‮ { ⁦*/\n    JavaScript/stretched-string.js:4:if (accessLevel != \"user‮ ⁦// Check if admin⁩ ⁦\") {\n    Python/commenting-out.py:4:if access_level != 'none‮⁦': # Check if admin ⁩⁦' and access_level != 'user\n    Python/early-return.py:5:    ''' Subtract funds from bank account then ⁧''' ;return\n    Rust/commenting-out.rs:3:    /*‮ } ⁦if is_admin⁩ ⁦ begin admins only */\n    Rust/commenting-out.rs:5:    /* end admins only ‮ { ⁦*/\n    Rust/stretched-string.rs:3:    if access_level != \"user‮ ⁦// Check if admin⁩ ⁦\" {\n    Binary file website/public/trojan-source.pdf matches\n    Binary file website/src/assets/img/faces/erik-lucatero-2.jpg matches\n    1\n\nThat \"Binary file...\" being because they don't maintain an up-to-date\n.gitattributes file.\n\nThe -e you've got is redundant since you have only one pattern. A relic\nof trying to feed a list of these with \"-e\" to \"git grep\"?\n\nBut anyway, as noted you're not correct that we don't have PCRE with the\n\"git\" in the CI, we just don't have \\uxxxx, but you don't need that\noptional syntax. Just use \\N{U+xxxx}:\n\n    ! git -P grep -nP '[\\N{U+202a}-\\N{U+202e}\\N{U+2066}-\\N{U+2069}]' ':(attr:!binary)'\n\nAll that being said I still don't see much of a point in doing this, but\nif we are let's not make it needlessly complex, but actually, why not\nsome variant of:\n    \n    $ ~/g/git/git grep -Pn '[\\N{U+2000}-\\N{U+206F}]' -- ':!(attr:binary)' ':!/po/' ':!*/po/*' ':!t/t0200/' ':!t/t0204*' ':!t/t5150-request-pull.sh'\n    builtin/gc.c:1543: *   * if the input value *cmd isn’t the key of any of the comma-separated list\n    builtin/gc.c:1555: *   GIT_TEST_MAINT_SCHEDULER set to “foo:./mock_foo.sh,bar:./mock_bar.sh”\n    builtin/submodule--helper.c:3262:                          N_(\"sets the submodule’s name to the given string \"\n    contrib/credential/netrc/git-credential-netrc.perl:122:   The credential’s username, if we already have one. (username=X)\n    t/helper/test-mergesort.c:364: * Function\" by Bentley and McIlroy, Software—Practice and Experience,\n    t/helper/test-mergesort.c:365: * Volume 23, Issue 11, 1249–1265 (November 1993).\n    Binary file t/t0013/shattered-1.pdf matches\n\nI.e. that notices that the \"binary\" is missing for that *.pdf, and that\nwe have some Unicode quoting we probably should get rid of anyway, it\nalso nicely excludes po/* in the pathspec, so any legitimate use of this\nfor RTL languages won't be prevented by this check.\n"},{"id":"440409","messageId":"xmqqk0ho4xo5.fsf@gitster.g","threadId":"56829","inReplyTo":"pull.1071.v2.git.1635942236065.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] ci: disallow directional formatting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-03T23:38:50Z","receivedAt":"2021-11-03T23:38:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n> +# U+2066..U+2069: LRI, RLI, FSI and PDI\n> +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n> +\n> +! git ls-files -z ':(attr:!binary)' |\n> +LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n\nOne thing for the future, and one thing for the present.\n\n - Do some languages we would add to po/ hierarchy in the future\n   possibly want to use these sequences as legitimate contents?\n\n - Do we need ls-files?  \n\n\nFor the latter, shouldn't the attribute-based pathspec work just\nfine with \"git grep\"?  i.e.\n\n    git grep -l -E -e $pattern -- ':(exclude,attr:binary)'\n\n"},{"id":"440444","messageId":"nycvar.QRO.7.76.6.2111041111570.56@tvgsbejvaqbjf.bet","threadId":"56829","inReplyTo":"xmqqk0ho4xo5.fsf@gitster.g","subject":"Re: [PATCH v2] ci: disallow directional formatting","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-04T10:19:08Z","receivedAt":"2021-11-04T10:19:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 3 Nov 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n> > +# U+2066..U+2069: LRI, RLI, FSI and PDI\n> > +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n> > +\n> > +! git ls-files -z ':(attr:!binary)' |\n> > +LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n>\n> One thing for the future, and one thing for the present.\n>\n>  - Do some languages we would add to po/ hierarchy in the future\n>    possibly want to use these sequences as legitimate contents?\n\nI mulled over that. And I think you're right. If a right-to-left\ntranslation needs to refer to, say, a `git` invocation, the part that\nshows the commandline surely would need to be guarded within directional\nformatting code points. We currently only have translated messages that\nread left-to-right, for example we lack Arabic and Hebrew translations.\nThose would be likely to contain such code points on purpose.\n\nI therefore added `:(exclude)*.po` to the command.\n\n>  - Do we need ls-files?\n>\n>\n> For the latter, shouldn't the attribute-based pathspec work just\n> fine with \"git grep\"?  i.e.\n>\n>     git grep -l -E -e $pattern -- ':(exclude,attr:binary)'\n\nD'oh. You're right! I fixed it.\n\nCiao,\nDscho\n"},{"id":"440450","messageId":"pull.1071.v3.git.1636031609982.gitgitgadget@gmail.com","threadId":"56829","inReplyTo":"pull.1071.v2.git.1635942236065.gitgitgadget@gmail.com","subject":"[PATCH v3] ci: disallow directional formatting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-04T13:13:29Z","receivedAt":"2021-11-04T13:13:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nAs described in https://trojansource.codes/trojan-source.pdf, it is\npossible to abuse directional formatting (a feature of Unicode) to\ndeceive human readers into interpreting code differently from compilers.\n\nFor example, an \"if ()\" expression could be enclosed in a comment, but\nrendered as if it was outside of that comment. In effect, this could\nfool a reviewer into misinterpreting the code flow as benign when it is\nnot.\n\nIt is highly unlikely that Git's source code wants to contain such\ndirectional formatting in the first place, so let's just disallow it.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    ci: disallow directional formatting\n    \n    A couple days ago, I stumbled over\n    https://siliconangle.com/2021/11/01/trojan-source-technique-can-inject-malware-source-code-without-detection/,\n    which details an interesting social-engineering attack: it uses\n    directional formatting in source code to pretend to human readers that\n    the code does something different than it actually does.\n    \n    It is highly unlikely that Git's source code wants to contain such\n    directional formatting in the first place, so let's disallow it.\n    \n    Technically, this is not exactly -rc material, but the paper was just\n    published, and I want us to be safe.\n    \n    Changes since v2:\n    \n     * The pathspec excluding binary files is now used directly instead of\n       doing the ls-files | xargs dance.\n     * Corrected a code comment: my custom git grep was not PCRE-enabled,\n       but Ubuntu's isn't. But git grep -P still does not understand \\uNNNN.\n     * Even if the *.po files currently pass the check, the script is now\n       future-proof by excluding them.\n     * Renamed the script to have the .bash extension, to indicate that it\n       requires a Bashism (i.e. a printf that understands the \\uNNNN\n       syntax).\n    \n    Changes since v1:\n    \n     * The code was moved into a script in ci/.\n     * We use git ls-files now to exclude files marked as binary in the Git\n       attributes.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1071%2Fdscho%2Fcheck-for-utf-8-directional-formatting-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1071/dscho/check-for-utf-8-directional-formatting-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1071\n\nRange-diff vs v2:\n\n 1:  bbf963695ba ! 1:  80447819de3 ci: disallow directional formatting\n     @@ .github/workflows/main.yml: jobs:\n           - uses: actions/checkout@v2\n           - run: ci/install-dependencies.sh\n           - run: ci/run-static-analysis.sh\n     -+    - run: ci/check-directional-formatting.sh\n     ++    - run: ci/check-directional-formatting.bash\n         sparse:\n           needs: ci-config\n           if: needs.ci-config.outputs.enabled == 'yes'\n      \n     - ## ci/check-directional-formatting.sh (new) ##\n     + ## ci/check-directional-formatting.bash (new) ##\n      @@\n      +#!/bin/bash\n      +\n     @@ ci/check-directional-formatting.sh (new)\n      +# to deceive reviewers into interpreting code differently from the compiler.\n      +# This is intended to run on an Ubuntu agent in a GitHub workflow.\n      +#\n     -+# `git grep` as well as GNU grep do not handle `\\u` as a way to specify UTF-8.\n     -+# A PCRE-enabled `git grep` would handle `\\u` as desired, but Ubuntu does\n     -+# not build its `git` packages with PCRE support.\n     ++# To allow translated messages to introduce such directional formatting in the\n     ++# future, we exclude the `.po` files from this validation.\n     ++#\n     ++# Neither GNU grep nor `git grep` (not even with `-P`) handle `\\u` as a way to\n     ++# specify UTF-8.\n      +#\n      +# To work around that, we use `printf` to produce the pattern as a byte\n      +# sequence, and then feed that to `git grep` as a byte sequence (setting\n     @@ ci/check-directional-formatting.sh (new)\n      +# U+2066..U+2069: LRI, RLI, FSI and PDI\n      +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n      +\n     -+! git ls-files -z ':(attr:!binary)' |\n     -+LC_CTYPE=C xargs -0r git grep -Ele \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" --\n     ++! LC_CTYPE=C git grep -El \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" \\\n     ++\t-- ':(exclude,attr:binary)' ':(exclude)*.po'\n\n\n .github/workflows/main.yml           |  1 +\n ci/check-directional-formatting.bash | 27 +++++++++++++++++++++++++++\n 2 files changed, 28 insertions(+)\n create mode 100755 ci/check-directional-formatting.bash\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 6ed6a9e8076..deda12db3a9 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -289,6 +289,7 @@ jobs:\n     - uses: actions/checkout@v2\n     - run: ci/install-dependencies.sh\n     - run: ci/run-static-analysis.sh\n+    - run: ci/check-directional-formatting.bash\n   sparse:\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\ndiff --git a/ci/check-directional-formatting.bash b/ci/check-directional-formatting.bash\nnew file mode 100755\nindex 00000000000..e6211b141aa\n--- /dev/null\n+++ b/ci/check-directional-formatting.bash\n@@ -0,0 +1,27 @@\n+#!/bin/bash\n+\n+# This script verifies that the non-binary files tracked in the Git index do\n+# not contain any Unicode directional formatting: such formatting could be used\n+# to deceive reviewers into interpreting code differently from the compiler.\n+# This is intended to run on an Ubuntu agent in a GitHub workflow.\n+#\n+# To allow translated messages to introduce such directional formatting in the\n+# future, we exclude the `.po` files from this validation.\n+#\n+# Neither GNU grep nor `git grep` (not even with `-P`) handle `\\u` as a way to\n+# specify UTF-8.\n+#\n+# To work around that, we use `printf` to produce the pattern as a byte\n+# sequence, and then feed that to `git grep` as a byte sequence (setting\n+# `LC_CTYPE` to make sure that the arguments are interpreted as intended).\n+#\n+# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n+# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n+# for example, would use a `printf` that does not understand that syntax.\n+\n+# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n+# U+2066..U+2069: LRI, RLI, FSI and PDI\n+regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n+\n+! LC_CTYPE=C git grep -El \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" \\\n+\t-- ':(exclude,attr:binary)' ':(exclude)*.po'\n\nbase-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n-- \ngitgitgadget\n"},{"id":"440453","messageId":"211104.86ee7whvoz.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"pull.1071.v3.git.1636031609982.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-04T13:48:29Z","receivedAt":"2021-11-04T13:55:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 04 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> [...]\n>      * Corrected a code comment: my custom git grep was not PCRE-enabled,\n>        but Ubuntu's isn't. But git grep -P still does not understand \\uNNNN.\n>      * Even if the *.po files currently pass the check, the script is now\n>        future-proof by excluding them.\n> [...]\n\n> +# This is intended to run on an Ubuntu agent in a GitHub workflow.\n> +#\n> +# To allow translated messages to introduce such directional formatting in the\n> +# future, we exclude the `.po` files from this validation.\n> +#\n> +# Neither GNU grep nor `git grep` (not even with `-P`) handle `\\u` as a way to\n> +# specify UTF-8.\n> +#\n> +# To work around that, we use `printf` to produce the pattern as a byte\n> +# sequence, and then feed that to `git grep` as a byte sequence (setting\n> +# `LC_CTYPE` to make sure that the arguments are interpreted as intended).\n> +#\n> +# Note: we need to use Bash here because its `printf` interprets `\\uNNNN` as\n> +# UTF-8 code points, as desired. Running this script through Ubuntu's `dash`,\n> +# for example, would use a `printf` that does not understand that syntax.\n> +\n> +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n> +# U+2066..U+2069: LRI, RLI, FSI and PDI\n> +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n> +\n> +! LC_CTYPE=C git grep -El \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" \\\n> +\t-- ':(exclude,attr:binary)' ':(exclude)*.po'\n>\n> base-commit: 0cddd84c9f3e9c3d793ec93034ef679335f35e49\n\nSo this is still using not-PCRE instead of the much simpler PCRE-enabled\none I suggested in[1] because your locally-built git doesn't link to\nlibpcre?\n\nAt the very least that comment is still quite confusing. I.e. it starts\nout by saying that it's meant to run in CI where we've got PCRE, but\nthen goes on to describe an elaborate workaround that's only needed with\na not-PCRE grep.\n\nWould be less confusing to understand why it's so complex as:\n\n   # This is intended to run on an Ubuntu agent in a GitHub\n   # workflow. We've got PCRE there, so all of the below could also be:\n   #\n   #    ! git -P grep -nP '[\\N{U+202a}-\\N{U+202e}\\N{U+2066}-\\N{U+2069}]' ':!(attr:binary)' ':!**po/**'\n   #\n   #\n   # But the author wanted to run this locally on a system that didn't\n   # have PCRE, So [go on to describe elaborate bash / \"git grep -E\" /\n   # LC_* tweaking workaround....]\n   [...]\n\nB.t.w. I think that **po/** exclusion is closer to what you want,\ni.e. \"a 'po' dir anywhere in our tree\". It'll also exclude this if we\ne.g. end up using these in language-specific README files there or\nwhatever.\n\n1. https://lore.kernel.org/git/211103.86zgqlhzvz.gmgdl@evledraar.gmail.com/ or:\n\n   ! git -P grep -nP '[\\N{U+202a}-\\N{U+202e}\\N{U+2066}-\\N{U+2069}]' ':!(attr:binary)'\n"},{"id":"440469","messageId":"xmqqpmrf3kjz.fsf@gitster.g","threadId":"56829","inReplyTo":"211104.86ee7whvoz.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3] ci: disallow directional formatting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-04T17:19:44Z","receivedAt":"2021-11-04T17:19:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n>> +# U+2066..U+2069: LRI, RLI, FSI and PDI\n>> +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n>> +\n>> +! LC_CTYPE=C git grep -El \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" \\\n>> +\t-- ':(exclude,attr:binary)' ':(exclude)*.po'\n> ...\n>    ! git -P grep -nP '[\\N{U+202a}-\\N{U+202e}\\N{U+2066}-\\N{U+2069}]' ':!(attr:binary)'\n\nSo you are comparing\n\n * requiring bash and C.UTF-8 locale to be available\n\nvs\n\n * requiring git built with PCRE\n\nassuming that \"Dscho says doesn't work with PCRE and you say it\nworks with PCRE\" is resolved?  They seem roughly the same\ndifficulty to me.\n\n"},{"id":"440670","messageId":"211108.86cznafpke.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"xmqqpmrf3kjz.fsf@gitster.g","subject":"Re: [PATCH v3] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-08T18:49:28Z","receivedAt":"2021-11-08T18:52:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 04 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>>> +# U+202a..U+2a2e: LRE, RLE, PDF, LRO and RLO\n>>> +# U+2066..U+2069: LRI, RLI, FSI and PDI\n>>> +regex='(\\u202a|\\u202b|\\u202c|\\u202d|\\u202e|\\u2066|\\u2067|\\u2068|\\u2069)'\n>>> +\n>>> +! LC_CTYPE=C git grep -El \"$(LC_CTYPE=C.UTF-8 printf \"$regex\")\" \\\n>>> +\t-- ':(exclude,attr:binary)' ':(exclude)*.po'\n>> ...\n>>    ! git -P grep -nP '[\\N{U+202a}-\\N{U+202e}\\N{U+2066}-\\N{U+2069}]' ':!(attr:binary)'\n>\n> So you are comparing\n>\n>  * requiring bash and C.UTF-8 locale to be available\n>\n> vs\n>\n>  * requiring git built with PCRE\n>\n> assuming that \"Dscho says doesn't work with PCRE and you say it\n> works with PCRE\" is resolved?  They seem roughly the same\n> difficulty to me.\n\nWe can hard depend on a git built with PCRE, since the point of this\nthing is to run in GitHub CI, Ubuntu builds git with PCRE, and that's\nunlikely to ever change.\n\nThe caveats around PCRE that still somewhat persist around PCRE are due\nto a misunderstanding, i.e. I think Johannes tried the \\uXXXX syntax,\nwhich we don't opt git-grep into, but as shown above you can just use\nthe other universally supported PCRE syntax for referring to Unicode\ncodepoints.\n"},{"id":"440679","messageId":"xmqqo86utnpi.fsf@gitster.g","threadId":"56829","inReplyTo":"211108.86cznafpke.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3] ci: disallow directional formatting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-08T20:08:25Z","receivedAt":"2021-11-08T20:08:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> So you are comparing\n>>\n>>  * requiring bash and C.UTF-8 locale to be available\n>>\n>> vs\n>>\n>>  * requiring git built with PCRE\n>>\n>> assuming that \"Dscho says doesn't work with PCRE and you say it\n>> works with PCRE\" is resolved?  They seem roughly the same\n>> difficulty to me.\n>\n> We can hard depend on a git built with PCRE, since the point of this\n> thing is to run in GitHub CI, Ubuntu builds git with PCRE, and that's\n> unlikely to ever change.\n\nYes, so is the availability of bash and C.UTF-8 for the same reason:\nwe are talking about controlled environment.  That is what I meant\nby \"roughly the same difficulty to me\".\n\nFWIW, I am OK with either approach, as I find the patch in question\nis just as readable as any rewrite that would use \"grep -P\", so...\n"},{"id":"440739","messageId":"211109.86bl2tjvns.gmgdl@evledraar.gmail.com","threadId":"56829","inReplyTo":"xmqqo86utnpi.fsf@gitster.g","subject":"Re: [PATCH v3] ci: disallow directional formatting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-09T13:34:12Z","receivedAt":"2021-11-09T13:39:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 08 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>>> So you are comparing\n>>>\n>>>  * requiring bash and C.UTF-8 locale to be available\n>>>\n>>> vs\n>>>\n>>>  * requiring git built with PCRE\n>>>\n>>> assuming that \"Dscho says doesn't work with PCRE and you say it\n>>> works with PCRE\" is resolved?  They seem roughly the same\n>>> difficulty to me.\n>>\n>> We can hard depend on a git built with PCRE, since the point of this\n>> thing is to run in GitHub CI, Ubuntu builds git with PCRE, and that's\n>> unlikely to ever change.\n>\n> Yes, so is the availability of bash and C.UTF-8 for the same reason:\n> we are talking about controlled environment.  That is what I meant\n> by \"roughly the same difficulty to me\".\n>\n> FWIW, I am OK with either approach, as I find the patch in question\n> is just as readable as any rewrite that would use \"grep -P\", so...\n\nTo each his own I guess :) I do find the simple regex of:\n\n    '[\\N{U+202a}-\\N{U+202e}\\N{U+2066}-\\N{U+2069}]'\n\nMuch easier to understand than something using printf, shell\ninterpolation, and needing to switch around LC_CTYPE to two different\nvalues twice on one line.\n\nBut anyway, that's a matter of taste.\n\nWhat isn't is the issue I noted at the bottom of [1], i.e. if we're\nrelying on '(attr:binary)' we should probably start with an assertion\nthat our idea of \"binary\" matches reality, or perhaps go back to the -I\nheuristic.\n\nBecause now we've got at least one binary non-attribute-marked file, and\nif files like that ever get updated they might start matching this\npattern. Maybe not a big deal, but someone updating those might\nconfusingly trip over this otherwise...\n\n1. https://lore.kernel.org/git/211103.86zgqlhzvz.gmgdl@evledraar.gmail.com/\n"}]}