build(verification): Verify compiler plugin dependencies with PGP (GRADLE-116) - #1423
Conversation
9003c05 to
e5361d2
Compare
…ADLE-116) The sentry-kotlin-compiler-plugin build resolved its dependencies with no verification at all. Enable it there, using signatures rather than checksums so that a routine version bump leaves the metadata untouched and any diff to it is rare enough to actually get read. Trust is 37 PGP keys covering everything whose publisher signs its artifacts; only 6 SHA-256 checksums remain, for the Gradle Plugin Portal marker POMs and a few unsigned parent POMs that ship no signature to verify. The keyring is armored so it diffs as text, and key servers are disabled. Two scopes Gradle's bootstrap inferred are deliberately narrowed, because both grant far more authority than the graph needs and neither is obvious on a later read: the JetBrains Compose key was generalised to all of org.jetbrains, which would let it vouch for Kotlin itself, and the Error Prone key to all of com.google, which would cover Guava, Gson and AutoService. Regeneration preserves existing entries, so both survive it; the header comment on the file says not to widen them back. Verification is scoped to the root of a build tree, so an included build's own configuration is ignored: preMerge does not exercise this, and the first job that does is the snapshot publish, which builds the compiler plugin from its own directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e5361d2 to
080a370
Compare
0xadam-brown
left a comment
There was a problem hiding this comment.
PGP sounds sensible here, and my clanker tells me the implementation closely follows Ritter's best practices. Nice 👍
A few comments for your consideration, but no blockers.
Your PR description says:
Verification is scoped to the build tree root, so an included build's own config is ignored — preMerge does not verify this build. No PR job exercises it; the first one that does is the snapshot publish, which builds the compiler plugin from its own directory.
Any interest in creating a PR job that runs the compiler plugin on its own (so we see failures before the PR merges)? Happy to defer to you...
| <trusted-key id="4797B4F5DCC46CEA61059071A1AE06236CA2BA62" group="com.diffplug.durian" name="durian-swt.os" version="4.3.0"/> | ||
| <trusted-key id="4DB1A49729B053CAF015CEE9A6ADFC93EF34893E" group="org.hamcrest"/> | ||
| <trusted-key id="60200AC4AE761F1614D6C46766D68DAA073BE985" group="org.slf4j"/> | ||
| <trusted-key id="648190996EC0930A6D7D49A978178478013521D0" group="com.facebook"/> |
There was a problem hiding this comment.
Thoughts about limiting to com.facebook:ktfmt, as Meta also publishes widely?
There was a problem hiding this comment.
That's a good idea. I'll do that in a follow up.
| ```bash | ||
| ./gradlew -p sentry-kotlin-compiler-plugin resolveAll spotlessCheck \ | ||
| --write-verification-metadata pgp,sha256 --export-keys | ||
| ``` |
There was a problem hiding this comment.
My clanker says this command still lets Gradle fetch missing keys during --write-verification-metadata unless it's run with --offline. Fwiw, that undermines our story about keys only being added deliberately.
You could consider adding --offline to the regeneration command or clarifying here that key servers might be hit unless performed offline. Happy to defer to you...
There was a problem hiding this comment.
That's either a hallucination or a bug in Gradle. Do you have any more information on how to reproduce or test this?
There was a problem hiding this comment.
Nope – it was just a comment I got consistently when asking my clanker if it had any concerns. Could very well be hallucinating. Feel free to disregard if you know better.
| - `sentry-kotlin-compiler-plugin` verifies its dependencies with PGP signatures | ||
| (`sentry-kotlin-compiler-plugin/gradle/verification-metadata.xml` plus an armored keyring). | ||
| Regenerate with | ||
| `./gradlew -p sentry-kotlin-compiler-plugin resolveAll spotlessCheck --write-verification-metadata pgp,sha256 --export-keys`, |
There was a problem hiding this comment.
Could be nice to add a small script so bots and humans don't goof.
There was a problem hiding this comment.
If its just one line, i don't think a script makes sense. We'll either have to copy and paste the script's name or copy and paste the one line and to me its the same.
…16) (#1430) * build(verification): Run compiler plugin verification in CI (GRADLE-116) Dependency verification is scoped to the root of a build tree, so `preMerge` from the repo root ignores the compiler plugin's own configuration. Nothing on a PR exercised it, and the first job that did was the snapshot publish on main. That gap already bit: #1426 bumped Spotless to 8.10.1 after #1423 branched, so #1423 merged pinning the 8.10.0 marker POM. The standalone build has been failing verification on main ever since, and the next snapshot publish would have been the first to notice. Regenerate the metadata and drop the dead 8.10.0 pin. Narrow the Ktfmt Team key from all of com.facebook to ktfmt and its parent POM. The parent needs covering too, otherwise every ktfmt bump adds a one-line component entry and the metadata stops being diff-free on version bumps. Also add the changelog entry #1423 skipped. * build(verification): Reference PR in changelog entry
Closes GRADLE-116. Fixes #1365, fixes #1366.
sentry-kotlin-compiler-pluginresolved its dependencies with no verification at all. This enables it, following Benedikt Ritter's best practices: PGP signatures rather than checksums, so a routine version bump leaves the metadata untouched and a diff here is rare enough to get read.failOnNonReproducibleResolution()— the graph was already fully pinnedRegeneration is a no-op against the committed file:
spotlessCheckis in that command because Spotless resolves ktfmt itself, as a task input — it lands in no declared configuration, soresolveAllalone leaves those 8 artifacts out of the metadata.Worth a look in review
Two scopes are deliberately narrower than what Gradle's bootstrap infers. Regeneration preserves them; the file header says not to widen them back.
org.jetbrains— would vouch for Kotlin itselfandroidx.lifecycle,skikocom.google— would cover Guava, Gson, AutoServiceerrorprone,googlejavaformatVerification is scoped to the build tree root, so an included build's own config is ignored —
preMergedoes not verify this build. No PR job exercises it; the first one that does is the snapshot publish, which builds the compiler plugin from its own directory.plugin-build's existing checksum metadata is untouched; converting it the same way can be a follow-up.Build-only change: #skip-changelog
🤖 Generated with Claude Code