Skip to content

chore: add gitguardian config to exclude compliance test key fixtures#33

Open
MichielDean wants to merge 1 commit into
mainfrom
chore/gitguardian-test-key-exclusions
Open

chore: add gitguardian config to exclude compliance test key fixtures#33
MichielDean wants to merge 1 commit into
mainfrom
chore/gitguardian-test-key-exclusions

Conversation

@MichielDean

Copy link
Copy Markdown
Collaborator

Summary

Add .gitguardian config to suppress false-positive secret detections on the AdCP compliance test vectors.

The test vectors in adcp-server/src/test/resources/compliance/ contain intentionally public test keys with _private_d_for_test_only fields. These ship in the official AdCP spec repo at https://adcontextprotocol.org/compliance/latest/test-vectors/ and are designed for cross-SDK conformance testing. The keys.json files carry explicit _WARNING and $comment banners.

GitGuardian correctly detects these as private key material, but they are false positives — the keys are deliberately public and must not be treated as secrets.

This PR adds a .gitguardian config with paths-ignore for the compliance test fixtures so that PR #32 (Track 4 L1 signing) and future PRs that reference these vectors don't trigger false-positive security alerts.

Note: This needs to be on main before the .gitguardian paths-ignore takes effect for PR checks.

The AdCP compliance test vectors in adcp-server/src/test/resources/compliance/
contain intentionally public test keys with _private_d_for_test_only fields.
These are for signer/verifier round-trip conformance testing and MUST NOT be
used in production. The keys.json files carry explicit warnings in their
_WARNING and  fields.

GitGuardian correctly detects these as private key material, but they are
false positives — the keys are published in the AdCP spec repo at
https://adcontextprotocol.org/compliance/latest/test-vectors/ and are
intentionally public for cross-SDK conformance testing.
@MichielDean MichielDean requested a review from bokelley as a code owner June 19, 2026 04:00
@aao-ipr-bot

aao-ipr-bot Bot commented Jun 19, 2026

Copy link
Copy Markdown

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

This is an automated message from the Argus AI review workflow.

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this needs another pass before approval.

Findings:

  1. .gitguardian does not configure the GitGuardian GitHub check run this PR is trying to quiet. GitGuardian's local ggshield config is documented as .gitguardian.yaml with version: 2 and secret.ignored_paths; the top-level paths-ignore / matches-ignore shape here is legacy ggshield config, and ggshield ignores are not shared with the GitGuardian dashboard/check-run integration. If the goal is to suppress the GitGuardian Security Checks status, this should be configured via the GitGuardian dashboard filepath exclusion rules, or the repo should add the actual ggshield config only if a local/CI ggshield invocation consumes it.

  2. The ignore patterns point at adcp-server/src/test/resources/compliance/..., but that path does not exist on either origin/main or this PR head. This makes the rule a no-op for the current repository state, even before considering whether GitGuardian will read the file. Please align the exclusion with the actual fixture path when those files are added, or include this config in the same PR that adds the fixtures so the pattern can be reviewed against real files.

One smaller security concern once the path issue is fixed: **/*hmac* is broad for a secret-scan bypass. Prefer excluding exact public test-vector files rather than every future compliance file whose name contains hmac.

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