this post was submitted on 12 Jul 2024
364 points (97.4% liked)

Programmer Humor

18961 readers
1190 users here now

Welcome to Programmer Humor!

This is a place where you can post jokes, memes, humor, etc. related to programming!

For sharing awful code theres also Programming Horror.

Rules

founded 1 year ago
MODERATORS
 

One does not commit or compile credentials

Template

Context:

This meme was brought to you by the PyPI Director of Infrastructure who accidentally hardcoded credentials - which could have resulted in compromissing the entire core Python ecosystem.

top 45 comments
sorted by: hot top controversial new old
[–] deegeese@sopuli.xyz 88 points 1 month ago (3 children)

If I had a dollar for every API key inside a config.json…

[–] marcos@lemmy.world 40 points 1 month ago (2 children)

Here's the thing, config.json should have been on the project's .gitignore.

Not exactly because of credentials. But, how do you change it to test with different settings?

[–] deegeese@sopuli.xyz 19 points 1 month ago

For a lot of my projects, there is a config-.json that is selected at startup based the environment.

Nothing secure in those, however.

[–] MajorHavoc@programming.dev 12 points 1 month ago* (last edited 1 month ago)

But, how do you change it to test with different settings?

When it's really messy, we:

  • check in a template file,
  • securely share a .env file (and .gitignore it)
  • and check in one line script that inflates the real config file (which we also .gitignore).
[–] MajorHavoc@programming.dev 19 points 1 month ago

I actually do have a dollar for every API key I or my team have committed inside a config file.

And...I'm doing pretty well.

Also, I've built some close friendships with our Cybersecurity team.

[–] fmstrat@lemmy.nowsci.com 5 points 1 month ago (1 children)

Can I have a dollar for every public S3 bucket?

[–] deegeese@sopuli.xyz 8 points 1 month ago* (last edited 1 month ago)

Might just make enough to pay your AWS bill this month.

[–] dan@upvote.au 52 points 1 month ago* (last edited 1 month ago) (4 children)

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)

[–] PlexSheep@infosec.pub 9 points 1 month ago (2 children)

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

[–] dan@upvote.au 7 points 1 month ago* (last edited 1 month 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.

[–] zqwzzle@lemmy.ca 4 points 1 month ago (1 children)

Depending on which stack you’re using, you could use https://danger.systems to automatically fail PRs.

[–] PlexSheep@infosec.pub 4 points 1 month ago (1 children)

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 2 points 1 month 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.

[–] cypherpunks@lemmy.ml 9 points 1 month ago (1 children)

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 1 points 1 month ago (1 children)

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

[–] cypherpunks@lemmy.ml 6 points 1 month 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.

[–] calcopiritus@lemmy.world 8 points 1 month 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 7 points 1 month ago (1 children)

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 3 points 1 month ago* (last edited 1 month ago) (1 children)

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.

[–] calcopiritus@lemmy.world 2 points 1 month ago

There are many git hooks. One of them checks each commit, but there's another that triggers on merges.

[–] carrylex@lemmy.world 47 points 1 month ago* (last edited 1 month ago) (5 children)

I also personally ask myself how a PyPI Admin & Director of Infrastructure can miss out on so many basic coding and security relevant aspects:

  • Hardcoding credentials and not using dedicated secret files, environment variable or other secret stores
  • For any source that you compile you have to assume that - in one way or another - it ends up in the final artifact - Apparently this was not fully understood (".pyc files containing the compiled bytecode weren't considered")
  • Not using a isolated build process e.g. a CI with an isolated VM or a container - This will inevitable lead to "works on my machine" scenarios
  • Needing the built artifact (containerimage) only locally but pushing it into a publicly available registry
  • Using a access token that has full admin permissions for everything, despite only requiring it to bypass rate limits
  • Apparently using a single access token for everything
    • When you use Git locally and want to push to GitHub you need an access token. The fact that article says "the one and only GitHub access token related to my account" likely indicates that this token was at least also used for this
  • One of the takeaways of the article says "set aggressive expiration dates for API tokens" - This won't help much if you don't understand how to handle them properly in the first place. An attacker can still use them before they expire or simply extract updated tokens from newer artifacts.

On the other hand what went well:

  • When this was reported it was reacted upon within a few minutes
  • Some of my above points of criticism now appear to be taken into account ("Takeaways")
[–] bleistift2@sopuli.xyz 28 points 1 month ago (1 children)

This will inevitable lead to “works on my machine” scenarios

Isn’t that what Python is all about?

[–] MajorHavoc@programming.dev 4 points 1 month ago

I feel seen.

[–] dohpaz42@lemmy.world 18 points 1 month ago (1 children)

Yes kids, the only stuff in ANY repo (public or otherwise) should be source code.

If it is compiled, built, or otherwise modified by any process outside of you the developer typing in your source code editor, it needs to be excluded/ignored from being committed. No excuses. None. Nope, not even that one.

No. 👏 Excuses. 👏

[–] bleistift2@sopuli.xyz 4 points 1 month ago

Two choices: Either the production software isn’t in the exact state the repo was when the software was built. Or I can’t get build timestamps in the software.

[–] dan@upvote.au 10 points 1 month ago

This will inevitable lead to "works on my machine" scenarios

Isn't this why Docker exists? It's "works on my machine"-as-a-service.

[–] Jayjader@jlai.lu 2 points 1 month ago

When you use Git locally and want to push to GitHub you need an access token.

I don't understand; I can push to GitHub using https creds or an ssh key without creating access tokens.

[–] cupcakezealot@lemmy.blahaj.zone 42 points 1 month ago (1 children)

don't commit credentials; split them up and place each part in a different place in the code and use code comments as a treasure map and make them work for it.

[–] dbx12@programming.dev 19 points 1 month ago

Ah, the horcrux technique.

[–] NigelFrobisher@aussie.zone 20 points 1 month ago

I mean, turns out it is pretty easy actually, Boromir.

[–] ArbitraryValue@sh.itjust.works 20 points 1 month ago* (last edited 1 month ago) (1 children)

On the contrary, one can commit or compile credentials quite simply... Maybe Boromir isn't the right person to ask.

[–] marcos@lemmy.world 15 points 1 month ago

Are you doubting Boromir's programming ability?

[–] lud@lemm.ee 12 points 1 month ago

Fun fact: if you search for "removed key" or something similar in GitHub you will get thousands of results of people removing accidentally committed keys. I'm guessing the vast majority of those removed keys haven't been revoked.

[–] MHanak@lemmy.world 5 points 1 month ago

This reminds me of that one time when i pushed with my github token as my username (dw i revoked it)

[–] leonard@social.menzel.lol 4 points 1 month ago (2 children)

@carrylex git should be password manager aware and refuse to commit if changes include a password

[–] carrylex@lemmy.world 9 points 1 month ago* (last edited 1 month ago) (2 children)

Well from my personal PoV there are a few problems with that

  1. You can't detect all credentials reliably, they could be encoded in base64 for example
  2. I think it's kind of okay to commit credentials and configuration used for the local dev environment (and ONLY the local one). E.g. when you require some infrastructure like a database inside a container for your app. Not every dev wants to manually set a few dozen configuration entries when they quickly want to checkout and run the app
[–] bleistift2@sopuli.xyz 15 points 1 month ago (1 children)

You can’t detect all credentials reliably,

Easy. You check in the password file first. Then you can check if the codebase contains any entry on the blacklist.

Wait…

[–] pfm@scribe.disroot.org 13 points 1 month ago (2 children)

You were so close! The right solution is of course training an AI model that detects credentials and rejects commits that contain them!

[–] tyler@programming.dev 7 points 1 month ago

You joke, but GitHub advanced security does this and more. On top of the AI component, they check the hash of all things that look like an api key and then also check them against their integrated vendors to see if they’re non-expired. I don’t know how well it works, but they claim like a .1% false positive rate or something like that.

[–] MajorHavoc@programming.dev 6 points 1 month ago

I need one of those reminder bots, so I can share a link to an inevitable startup, six months from now, based on your humorous comment.

[–] dohpaz42@lemmy.world 10 points 1 month ago

I think it's kind of okay to commit credentials and configuration used for the local dev environment (and ONLY the local one).

No. Never.

E.g. when you require some infrastructure like a database inside a container for your app. Not every dev wants to manually set a few dozen configuration entries when they quickly want to checkout and run the app

In this situation, it would be better to write a simple script that can generate fresh and unique values for the dev.

Laziness is not an excuse.

[–] dohpaz42@lemmy.world 6 points 1 month ago (1 children)

They do. But, as they say,ake it idiot-proof, and someone will make a better idiot.

[–] docAvid@midwest.social 14 points 1 month ago (1 children)
[–] dohpaz42@lemmy.world 5 points 1 month ago

You’re right. I do that sometimes.