From 2e5a51b54fa59904435f74c36d6a7cdce61f6577 Mon Sep 17 00:00:00 2001 From: Max Lambrecht Date: Wed, 5 Aug 2026 12:50:39 -0500 Subject: [PATCH] chore(github): add copilot PR review instructions Signed-off-by: Max Lambrecht --- .github/copilot-instructions.md | 21 +++++++++++++ .github/instructions/build.instructions.md | 15 ++++++++++ .../docs-and-examples.instructions.md | 17 +++++++++++ .github/instructions/helper.instructions.md | 16 ++++++++++ .github/instructions/provider.instructions.md | 22 ++++++++++++++ .github/instructions/review.instructions.md | 11 +++++++ .github/instructions/security.instructions.md | 23 ++++++++++++++ .../instructions/spiffe-id.instructions.md | 19 ++++++++++++ .../svid-and-bundle.instructions.md | 30 +++++++++++++++++++ .github/instructions/tests.instructions.md | 9 ++++++ .../instructions/workload-api.instructions.md | 18 +++++++++++ 11 files changed, 201 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/build.instructions.md create mode 100644 .github/instructions/docs-and-examples.instructions.md create mode 100644 .github/instructions/helper.instructions.md create mode 100644 .github/instructions/provider.instructions.md create mode 100644 .github/instructions/review.instructions.md create mode 100644 .github/instructions/security.instructions.md create mode 100644 .github/instructions/spiffe-id.instructions.md create mode 100644 .github/instructions/svid-and-bundle.instructions.md create mode 100644 .github/instructions/tests.instructions.md create mode 100644 .github/instructions/workload-api.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..a779c773 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,21 @@ +# Copilot Instructions + +Repository instructions for Copilot: + +- Before reviewing or suggesting edits, inspect nearby code, tests, and README files. +- Prefer minimal patches that fit the current design. +- In reviews, focus on correctness, security, spec alignment, API consistency, and missing tests. +- Do not invent repository conventions. Infer them from existing code. +- Do not suggest broad refactors unless the prompt explicitly asks for them. +- For public API changes, call out compatibility impact explicitly. +- For security-sensitive code, explain assumptions and failure modes. +- When unsure, say what is uncertain instead of guessing. + +For pull request reviews: + +- Lead with the highest-severity finding, and keep the review to a few high-signal comments. +- Path-specific expectations live in `.github/instructions/`. Apply the ones matching the changed files. +- Ground every finding in the actual diff, nearby code, existing tests, or the official SPIFFE standards when accessible; do not report speculative or generic issues. +- For spec-sensitive findings, cite the relevant SPIFFE standard or classify the issue as an ambiguity instead of asserting a violation. +- For each finding, state the concrete impact or failure scenario and the smallest reasonable fix, not just a theoretical risk. +- Do not invent findings to fill space. If there are no actionable issues, say so briefly instead of adding unnecessary comments or praise. diff --git a/.github/instructions/build.instructions.md b/.github/instructions/build.instructions.md new file mode 100644 index 00000000..d3c9319b --- /dev/null +++ b/.github/instructions/build.instructions.md @@ -0,0 +1,15 @@ +--- +applyTo: "**/build.gradle,**/settings.gradle,**/gradle.properties,**/module-info.java" +--- + +This is a multi-module Gradle build (`java-spiffe-core`, `java-spiffe-provider`, `java-spiffe-helper`) published to Maven Central, targeting Java 8 source/target compatibility. + +Check: +- dependency version bumps for known CVEs, license changes, and transitive impact across modules +- the pinned relationship between the gRPC version and the shaded Netty version in the root `build.gradle`; a gRPC bump without a matching Netty check is a likely bug +- new dependencies added to a module that should stay dependency-light (especially `java-spiffe-core`) +- changes that would raise the minimum Java version, since the project targets Java 8 +- `module-info.java` changes for exported/required packages that would change the public module surface +- publishing configuration (`com.vanniktech.maven.publish`, signing, artifact coordinates) for accidental changes to what gets published or how it is versioned + +Flag any dependency or build change without a stated reason, and any change that could silently alter what is published to Maven Central. diff --git a/.github/instructions/docs-and-examples.instructions.md b/.github/instructions/docs-and-examples.instructions.md new file mode 100644 index 00000000..7b0efff7 --- /dev/null +++ b/.github/instructions/docs-and-examples.instructions.md @@ -0,0 +1,17 @@ +--- +applyTo: "**/*.md,**/examples/**" +--- + +# Documentation And Examples + +Review documentation and examples for correctness against the current code and SPIFFE semantics. + +Check: + +- public API names, package names, artifact names, and module names match the implementation +- examples preserve security-sensitive defaults and do not weaken verification, trust-domain checks, or provider behavior +- commands, Gradle snippets, environment variables, socket paths, keystore/truststore paths, and passwords are clearly scoped to examples or local testing +- for SPIFFE semantics, prefer the official standards at `https://github.com/spiffe/spiffe/tree/main/standards`; do not rewrite normative language casually +- README or example changes mention compatibility, configuration, or behavior changes when the corresponding code changed + +Flag docs that teach unsafe patterns, expose secrets, imply production defaults for test-only settings, or drift from the public API. diff --git a/.github/instructions/helper.instructions.md b/.github/instructions/helper.instructions.md new file mode 100644 index 00000000..5557b622 --- /dev/null +++ b/.github/instructions/helper.instructions.md @@ -0,0 +1,16 @@ +--- +applyTo: "**/java-spiffe-helper/**" +--- + +This module writes SVIDs and bundles into Java keystores and truststores on disk. + +Check: +- keystore versus truststore separation, so private keys never land in the truststore +- entry aliasing and whether a refresh replaces, duplicates, or orphans prior entries +- file paths, permissions, and passwords read from configuration +- behavior when the Workload API is unavailable at startup or during refresh +- shutdown behavior and cleanup of partially written stores + +Flag passwords or key material that reach logs or exception messages, and refresh paths that leave a store in a partially updated state. + +Helper concerns must not leak into `java-spiffe-core` APIs. diff --git a/.github/instructions/provider.instructions.md b/.github/instructions/provider.instructions.md new file mode 100644 index 00000000..40c2b603 --- /dev/null +++ b/.github/instructions/provider.instructions.md @@ -0,0 +1,22 @@ +--- +applyTo: "**/java-spiffe-provider/**" +--- + +This code integrates with Java Security Provider mechanics. + +Be careful with: +- provider semantics +- keystore/truststore behavior +- initialization order +- exception clarity +- backward compatibility + +Specifically check: +- provider and algorithm registration, and any reliance on registration order or position +- `KeyManager` and `TrustManager` behavior when no SVID or bundle is available yet +- SPIFFE ID acceptance checks in the trust manager, including how the accepted-ID list is sourced and refreshed +- separation between identity material in the keystore and trust material in the truststore +- static or global provider state that can leak across callers, classloaders, or tests +- system property and configuration defaults that silently change a trust decision + +Do not suggest changes that make provider behavior more implicit or harder to reason about. Behavioral provider changes need tests for initialization failure and for rejection of untrusted peer identities; this does not apply to docs, comments, or non-behavioral cleanup. \ No newline at end of file diff --git a/.github/instructions/review.instructions.md b/.github/instructions/review.instructions.md new file mode 100644 index 00000000..19ebc6b7 --- /dev/null +++ b/.github/instructions/review.instructions.md @@ -0,0 +1,11 @@ +--- +applyTo: "**" +--- + +For code review tasks: +- Prioritize concrete correctness and security findings over style nitpicks. +- Do not stop at style or readability. Look first for correctness bugs, security risks, spec mismatches, backward-compatibility issues, API inconsistencies, missing edge-case tests, and subtle lifecycle/concurrency problems. +- Point to exact risks, not vague concerns. +- Call out missing edge-case coverage, missing negative tests, and backward-compatibility risks. +- Flag changes that alter public API, validation semantics, trust decisions, refresh behavior, or error contracts. +- Prefer a few high-signal comments over many low-value comments. \ No newline at end of file diff --git a/.github/instructions/security.instructions.md b/.github/instructions/security.instructions.md new file mode 100644 index 00000000..2014f3e6 --- /dev/null +++ b/.github/instructions/security.instructions.md @@ -0,0 +1,23 @@ +--- +applyTo: "**/io/spiffe/**,**/java-spiffe-provider/**,**/java-spiffe-helper/**" +--- + +This code makes trust decisions. Review it as security-sensitive. + +For spec-sensitive security findings, use the official SPIFFE standards as the source of truth when accessible: +https://github.com/spiffe/spiffe/tree/main/standards + +Prioritize findings in this order: +1. Invalid identities, SVIDs, bundles, or JWTs that could be accepted. +2. Valid material rejected in a way that breaks existing callers. +3. Trust-domain, issuer, audience, key, or bundle confusion. +4. Stale or partially applied updates that remain usable for authorization. +5. Identity material (private keys, SVIDs) mixed with trust material (bundles, CA certs). +6. Failures that do not fail closed, or that swallow the original cause. + +Also flag: +- Private keys, tokens, passwords, keystores, or truststores written to logs, exceptions, temp files, or test fixtures. +- Validation moved behind a flag, cached, or short-circuited. +- New defaults that make a trust decision implicitly. + +Do not report generic hardening advice that is unrelated to the diff. diff --git a/.github/instructions/spiffe-id.instructions.md b/.github/instructions/spiffe-id.instructions.md new file mode 100644 index 00000000..35bc70cb --- /dev/null +++ b/.github/instructions/spiffe-id.instructions.md @@ -0,0 +1,19 @@ +--- +applyTo: "**/io/spiffe/spiffeid/**" +--- + +This package implements SPIFFE ID and trust domain semantics. Treat the official SPIFFE ID standard as ground truth: +https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md + +Check that changes preserve: +- scheme `spiffe`, compared case-insensitively +- trust domain taken from the authority host, canonicalized to lowercase +- rejection of userinfo, port, query, fragment, and percent-encoding +- rejection of empty hosts and IPv6 authority forms +- path case sensitivity, with no silent normalization of segments +- rejection of empty, `.`, and `..` segments and trailing slashes +- equality and canonical string rendering staying consistent with each other + +Flag any change that makes parsing more permissive, or that adds strictness beyond the spec without calling it out as a deliberate compatibility change. + +Parsing changes need both accepting and rejecting test cases. diff --git a/.github/instructions/svid-and-bundle.instructions.md b/.github/instructions/svid-and-bundle.instructions.md new file mode 100644 index 00000000..c9bf2db2 --- /dev/null +++ b/.github/instructions/svid-and-bundle.instructions.md @@ -0,0 +1,30 @@ +--- +applyTo: "**/io/spiffe/svid/**,**/io/spiffe/bundle/**" +--- + +This code parses and validates X.509-SVIDs, JWT-SVIDs, and bundles. Use the official SPIFFE standards when a finding depends on spec semantics: +- https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md +- https://github.com/spiffe/spiffe/blob/main/standards/X509-SVID.md +- https://github.com/spiffe/spiffe/blob/main/standards/JWT-SVID.md +- https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Trust_Domain_and_Bundle.md + +For X.509-SVIDs, check: +- URI SAN extraction and behavior when there are zero or multiple URI SANs +- chain ordering assumptions and leaf versus CA semantics +- bundle selection by trust domain, and behavior on trust-domain mismatch +- expiry and malformed certificate handling + +For JWT-SVIDs, check: +- audience, subject, issuer, expiry, and not-before validation +- algorithm and key ID handling, and how a key is selected from the bundle +- behavior when the bundle has multiple, missing, or unusable keys +- that signature verification cannot be skipped for any token shape + +For bundles, check: +- parsing of incomplete or inconsistent key material +- bundle replacement versus merge semantics on update +- federation and cross-trust-domain lookup behavior + +Flag validation that is reordered, skipped on a fast path, or turned into a warning. Every validation change needs a negative test with malformed material. + +If a change under `io/spiffe/internal/` supports SVID, JWT, or bundle parsing (for example key or algorithm handling used during validation), apply this same checklist to it. diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md new file mode 100644 index 00000000..75ce309b --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,9 @@ +--- +applyTo: "**/*test*.*,**/tests/**,**/src/test/**" +--- + +When editing or generating tests: +- Keep tests deterministic and readable. +- Test behavior, not implementation details, unless the repository already uses white-box tests for a good reason. +- Add focused assertions for edge cases, invalid input, refresh/update behavior, and error messages when they matter. +- Avoid brittle timing-dependent tests unless the repository already has a pattern for controlling time. \ No newline at end of file diff --git a/.github/instructions/workload-api.instructions.md b/.github/instructions/workload-api.instructions.md new file mode 100644 index 00000000..9fc604c3 --- /dev/null +++ b/.github/instructions/workload-api.instructions.md @@ -0,0 +1,18 @@ +--- +applyTo: "**/io/spiffe/workloadapi/**" +--- + +This code owns Workload API clients and the long-lived `X509Source` and `JwtSource` objects. Use the official Workload API standard when a finding depends on spec semantics: +https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Workload_API.md + +Check: +- fetch versus watch semantics, and what callers observe before the first update +- whether an update replaces prior state atomically, without exposing a partial view +- stale data after a failed update, reconnect, cancellation, or close +- reconnect and retry behavior, including whether errors reach the caller or are silently swallowed +- stream observer lifecycle, executor shutdown, and resource cleanup on close +- thread safety of shared source objects and their internal caches + +Flag any change that lets a closed or failed source keep serving identities, or that turns a hard failure into a silent fallback. + +Lifecycle and concurrency changes need deterministic tests. Do not accept new tests that depend on sleeps for timing.