Skip to content

hooks: catch OpenAI project, service-account and admin keys - #21

Merged
StayLameBro merged 1 commit into
StayLameBro:mainfrom
kevin9327:fix/openai-project-keys
Oct 6, 2026
Merged

StayLameBro merged 1 commit into
StayLameBro:mainfrom
kevin9327:fix/openai-project-keys

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

check-sensitive.py lists OpenAI among the secrets it catches, but its OpenAI pattern only matches older key formats:

sk-(proj-)?[A-Za-z0-9]{32,}

That wants 32 plain alphanumerics right after the prefix. Project keys (sk-proj-…) put - and _ in their body, so a key is missed whenever one of those shows up in its first 32 characters, which is common. Service-account (sk-svcacct-…) and admin (sk-admin-…) keys never match.

So a commit that adds one of these keys is accepted by the pre-commit / pre-push hooks.

Change:

  • Prefixed OpenAI keys (proj, svcacct, admin) accept [A-Za-z0-9_-] in the body.
  • Legacy sk- keys keep the alphanumeric-only rule, so kebab-case text like sk-learn-… still does not match.

Tests: three expect_block cases in tests/security/check-sensitive-test.sh (project, service-account, admin), assembled at runtime like the existing Anthropic one so this file stays clean. All three fail on main (the key is committed) and pass with this change.

bash tests/security/check-sensitive-test.sh
# with the fix: 44 passed (1 remaining failure is `stat -f %Lp`, macOS-only, unrelated)
# without the fix: the three new OpenAI cases fail with "… was committed"

Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>

@StayLameBro StayLameBro left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this! The old pattern let many project keys, and every service-account and admin key, through the hooks, so this closes a real gap. Keeping legacy sk- keys alphanumeric-only so text like sk-learn-… doesn't trip it is a nice touch, and building the test keys at runtime keeps the test file clean.

@StayLameBro
StayLameBro merged commit 8130c09 into StayLameBro:main Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants