• dan@upvote.au
    link
    fedilink
    arrow-up
    53
    ·
    edit-2
    4 months ago

    At my workplace, we use the string @nocommit to designate code that shouldn’t be checked in. Usually in a comment:

    // @nocommit temporary for testing
    apiKey = 'blah';
    // apiKey = getKeyFromKeychain(); 
    

    but it can be anywhere in the file.

    There’s a lint rule that looks for @nocommit in all modified files. It shows a lint error in dev and in our code review / build system, and commits that contain @nocommit anywhere are completely blocked from being merged.

    (the code in the lint rule does something like "@no"+"commit" to avoid triggering itself)

    • Arthur Besse@lemmy.ml
      link
      fedilink
      English
      arrow-up
      9
      ·
      4 months ago

      At my workplace, we use the string @nocommit to designate code that shouldn’t be checked in

      That approach seems useful but it wouldn’t have prevented the PyPI incident OP links to: the access token was temporarily entered in a .py python source file, but it was not committed to git. The leak was via .pyc compiled python files which made it into a published docker build.

      • OhNoMoreLemmy@lemmy.ml
        link
        fedilink
        arrow-up
        1
        ·
        4 months ago

        Yeah, but a combination of this approach, and adding all compiled file types including .pyc to .gitignore would fix it.

        • Arthur Besse@lemmy.ml
          link
          fedilink
          English
          arrow-up
          6
          ·
          4 months ago

          adding all compiled file types including .pyc to .gitignore would fix it

          But in this case they didn’t accidentally put the token in git; the place where they forgot to put *.pyc was .dockerignore.

    • PlexSheep@infosec.pub
      link
      fedilink
      arrow-up
      9
      ·
      4 months ago

      This sounds like a really useful solution, how do you implement something like this? Especially with linter integration

      • dan@upvote.au
        link
        fedilink
        arrow-up
        7
        ·
        edit-2
        4 months ago

        I’m not sure, sorry. The source control team at work set it up a long time ago. I don’t know how it works - I’m just a user of it.

        The linter probably just runs git diff | grep @nocommit or similar.

        • PlexSheep@infosec.pub
          link
          fedilink
          arrow-up
          4
          ·
          4 months ago

          PRs? Isn’t the point of @nocommit that something does not get committed, and therefore no credentials are stored in the git repository? Even if the PR does not get merged, the file is still stored as a hit object and can be restored.

          • zqwzzle@lemmy.ca
            link
            fedilink
            English
            arrow-up
            2
            ·
            4 months ago

            I read the lint part and my brain forgot about everything else. You could stick the danger call in a pre commit hook though.

    • calcopiritus@lemmy.world
      link
      fedilink
      arrow-up
      8
      ·
      4 months ago

      This is a huge idea. I’m stealing it.

      Not just for credentials, there are many times where I change a setting or whatever and just put “//TODO: remember to set it back to ‘…’ before commiting”. I forget to change it back 99% of the time.

    • 8uurg@lemmy.world
      link
      fedilink
      arrow-up
      7
      ·
      4 months ago

      Neat idea. This could be refined by adding a git hook that runs (rip)grep on the entire codebase and fails if anything is found upon commit may accomplish a similar result and stop the code from being committed entirely. Requires a bit more setup work on de developers end, though.

      • dan@upvote.au
        link
        fedilink
        arrow-up
        3
        ·
        edit-2
        4 months ago

        Would a git hook block you from committing it locally, or would it just run on the server side?

        I’m not sure how our one at work is implemented, but we can actually commit @nocommit files in our local repo, and push them into the code review system. We just can’t merge any changes that contain it.

        It’s used for common workflows like creating new database entities. During development, the ORM system creates a dev database on a test DB cluster and automatically points the code for the new table to it, with a @nocommit comment above it. When the code is approved, the new schema is pushed to prod and the code is updated to point to the real DB.

        Also, the codebase is way too large for something like ripgrep to search the whole codebase in a reasonable time, which is why it only searches the commit diffs themselves.