Added a vendored version of httpsig-go.#30820
Conversation
📝 WalkthroughWalkthroughA new third-party Go library, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Signer
participant HTTPServer
participant Verifier
participant KeyFetcher
Client->>Signer: Prepare HTTP request
Signer->>Signer: Compute signature base
Signer->>Signer: Sign request (private key/secret)
Signer->>Client: Return signed request
Client->>HTTPServer: Send signed HTTP request
HTTPServer->>Verifier: Pass request for verification
Verifier->>KeyFetcher: Fetch public key/secret by key ID
KeyFetcher-->>Verifier: Return key info
Verifier->>Verifier: Compute signature base
Verifier->>Verifier: Verify signature
Verifier->>HTTPServer: Return verification result
HTTPServer->>Client: Respond (based on verification)
sequenceDiagram
participant Dev
participant CI
participant CodeQL
participant GoTest
Dev->>CI: Push code or PR
CI->>CodeQL: Run static analysis
CI->>GoTest: Run Go tests
CodeQL-->>CI: Report analysis results
GoTest-->>CI: Report test results
CI->>Dev: Show build and test status
If you want to learn more about HTTP signatures, you can read RFC 9421. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #30820 +/- ##
==========================================
- Coverage 64.18% 64.18% -0.01%
==========================================
Files 1888 1888
Lines 185001 185001
Branches 5324 5324
==========================================
- Hits 118747 118745 -2
Misses 56920 56920
- Partials 9334 9336 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (3)
third_party/httpsig-go/testdata/request1.txt (1)
1-9: same content noted earliersee the comment on
rfc-test-request.txt; the duplication note applies here as well.third_party/httpsig-go/testdata/test-key-ecc-p256.key (1)
1-5: repeat of private-key concernsame guidance as for the RSA-PSS key: mark clearly as test-only and update your secret-scanning allow-list so CI stays green.
third_party/httpsig-go/testdata/verify_request1.txt (1)
1-10: duplicate of b21 vectorThis file is byte-for-byte identical to
b21_request_signed.txt. If the duplication is deliberate for clarity, all good; otherwise consider referencing a single source of truth.
🧹 Nitpick comments (34)
third_party/httpsig-go/testdata/fuzz/FuzzSign/01c151dfb31ce1b1 (1)
1-4: add a brief header commentA one-line comment saying “autogenerated go-fuzz seed corpus” clarifies the purpose and keeps tooling from wondering what the file is.
third_party/httpsig-go/testdata/rfc-test-request.txt (1)
1-9: possible duplicate withrequest1.txtthis request payload is byte-for-byte identical to
third_party/httpsig-go/testdata/request1.txt. keeping two copies can confuse future readers and makes maintenance a little harder. consider removing one of them or turning one into a symlink.third_party/httpsig-go/testdata/b25_request_sigbase.txt (1)
1-4: add a trailing newlinemany tools warn when text files lack a final newline, and git diffs show a red ⏎ marker. adding one prevents needless churn.
-"@signature-params": ("date" "@authority" "content-type");created=1618884473;keyid="test-shared-secret" +"@signature-params": ("date" "@authority" "content-type");created=1618884473;keyid="test-shared-secret" +third_party/httpsig-go/.sideignore (2)
3-4: Typo in commentLine 3 repeats the word “to”.
-# the same as .gitignore. Note that any patterns listed here are ignored *in -# addition* to to what is ignored via .sideignore. +# the same as .gitignore. Note that any patterns listed here are ignored *in +# addition* to what is ignored via .sideignore.
15-19: Duplicate ignore patterns
vendor/appears twice (15 and 28) anddeps/twice (18 and 42). This is harmless but noisy.-15 vendor/ -28 vendor/ -18 deps/ -41 deps/ +15 vendor/ +18 deps/Also applies to: 28-29, 41-42
third_party/httpsig-go/.github/workflows/codeql.yml (1)
14-20: Consider path filtering to avoid scanning the entire monorepoBecause this vendored copy lives in
third_party/httpsig-go/, every push tomainwill trigger a full-repo CodeQL run. Adding apaths:filter keeps CI noise and runtime down.on: push: branches: [ "main" ] paths: - "third_party/httpsig-go/**" pull_request: branches: [ "main" ] paths: - "third_party/httpsig-go/**" schedule: - cron: '42 20 * * 0'third_party/httpsig-go/testdata/fuzz/FuzzSign/2da5aa149625a656 (1)
2-4: Duplicate seed value
"0"appears twice; duplicates do not add coverage.-string("0")third_party/httpsig-go/side.toml (1)
4-5: Run all packages during testsIf the module contains nested packages,
go test ./...is safer thango test.- command = "go test" + command = "go test ./..."third_party/httpsig-go/testdata/fuzz/FuzzSign/58358ab8a686cc26 (1)
1-4: Consider augmenting the fuzz corpusThis seed is a good minimal case, but adding a few more edge-case inputs (e.g., very long strings, Unicode, invalid UTF-8) will exercise the signer more thoroughly and speed-up bug discovery.
third_party/httpsig-go/testdata/fuzz/FuzzSign/ff08bb6e0efa59be (1)
1-4: Nice to have: broaden fuzz input diversitySimilar to the other corpus file, consider complementing these short strings with inputs that stress base-64 parsing, oversized headers, or unusual control characters to maximise coverage.
third_party/httpsig-go/go.mod (1)
1-8: Verify build wiring for the vendored moduleBecause this vendored copy declares its own module path
github.com/remitly-oss/httpsig-go, Go tooling will prefer the remote module unless the rootgo.modadds areplacedirective pointing to this local path. Without it, the new code may be silently ignored or drift from upstream.Example change to the root
go.mod(outside this file):+replace github.com/remitly-oss/httpsig-go => ./third_party/httpsig-goPlease add the directive (and
go mod tidy) to make sure CI and consumers actually pick up the vendored code.third_party/httpsig-go/testdata/test-key-rsa.key (1)
1-26: duplicate rsa key – same checks applysame considerations as previous file. if possible, keep only one representative rsa key to reduce repo size, otherwise ensure they’re all scoped to test-only code.
third_party/httpsig-go/testdata/fuzz/FuzzExtractSignatures/00e15d22123489fd (1)
1-4: nice to seed the fuzzersmall nit: add a short
READMEin thefuzz/dir describing the corpus layout so future maintainers know how to regenerate/update seeds.third_party/httpsig-go/testdata/fuzz/FuzzExtractSignatures/85649d45641911d0 (1)
1-4: same doc comment request for fuzz corpusa single
READMEcan cover both seeds.third_party/httpsig-go/testdata/test-key-ed25519.key (1)
1-3: gitleaks flag: confirm this key is non-sensitivegitleaks spotted a private key. since this is test data, that’s expected, but please:
- mark the path in
.gitattributeswithexport-ignore(or equivalent) so it’s excluded from release tarballs.- add a brief comment in the directory noting these keys are dummy test fixtures.
happy to draft the gitattributes snippet if you’d like.
third_party/httpsig-go/testdata/rfc-test-response.txt (1)
1-8: consider adding a trailing newlinesome tooling complains if a text file does not end with
\n. it looks like the file currently ends right after the closing brace. adding a newline keeps diffs andcatoutput tidy.third_party/httpsig-go/keyutil/testdata/test-jwk-ec.json (1)
1-8: flag as test-only datathis jwk contains a full private scalar (
"d"). be sure it’s used exclusively inside the test harness and never loaded in production code. a short comment at the top of the file (or aREADMEin the directory) explaining its purpose helps future readers and quiets security scanners.third_party/httpsig-go/keyutil/testdata/test-jwk-symmetric.json (1)
1-5: flag test-only key material clearlythis file embeds a raw symmetric key. that’s ok for unit tests, yet dangerous if someone accidentally re-uses it in prod. please add an inline comment or rename the file with a clear
*_testonly.jsonsuffix so future readers do not copy-paste it into live code.third_party/httpsig-go/.github/workflows/wf.yml (1)
1-15: tighten ci: add caching and static analysisadding two tiny steps will speed up jobs and catch bugs early.
test: runs-on: ubuntu-latest steps: + - name: enable go module cache + uses: actions/cache@v4 + with: + path: | + ~/.cache/go-build + ${{ env.GOMODCACHE }} + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: ${{ runner.os }}-go- - name: Set up Go uses: actions/setup-go@v4 with: go-version: '1.21' - name: Checkout code uses: actions/checkout@v3 + - name: vet + run: go vet ./... - name: Test - run: go test -v ./... + run: go test -race -v ./...cache speeds jobs,
go vetand-racesurface common defects.third_party/httpsig-go/README.md (4)
10-10: Fix spelling of “proof-of-possession”.Small typo sneaked in. Let’s tidy it up for clarity.
-HTTP signatures can be (or will be able to) used for demonstrating proof-of-posession ([DPoP]... +HTTP signatures can be (or will be able to) used for demonstrating proof-of-possession ([DPoP]...
19-19: Correct “repsonses” → “responses”.-Create net/http clients that sign requests and/or verifies repsonses. +Create net/http clients that sign requests and/or verifies responses.
13-14: Consider trimming the phrase “with the exception of”.The sentence is a bit wordy. Something like “All parts of the spec are supported except:” reads smoother.
21-53: Replace hard tabs with spaces to satisfy markdownlint.Tabs trigger MD010 violations and can render inconsistently. Four spaces keeps tools and eyeballs happy.
Example (showing the first few lines):
- params := httpsig.SigningOptions{ - PrivateKey: nil, // Fill in your private key + params := httpsig.SigningOptions{ + PrivateKey: nil, // Fill in your private keyApply the same conversion throughout the code blocks.
third_party/httpsig-go/keyman/keyman.go (2)
27-27: Consider lowercasing error message to follow Go conventions.Error messages in Go should typically start with lowercase letters unless they begin with proper nouns or acronyms.
- return nil, fmt.Errorf("Key for keyid '%s' not found", keyID) + return nil, fmt.Errorf("key for keyid '%s' not found", keyID)
33-33: Consider lowercasing error message to follow Go conventions.Error messages in Go should typically start with lowercase letters unless they begin with proper nouns or acronyms.
- return nil, fmt.Errorf("Fetch without keyid not supported") + return nil, fmt.Errorf("fetch without keyid not supported")third_party/httpsig-go/sigtest/helpers.go (1)
79-85: Consider consolidating the duplicatedDifffunction.The
Difffunction appears to be duplicated across multiple test files (also inkeyutil/jwk_test.go). Consider moving this common utility to a shared location to follow the DRY principle and make maintenance easier.// Consider moving this to a shared test utilities package // or keeping it only in the sigtest package for reusethird_party/httpsig-go/fz_test.go (1)
30-30: Consider optimizing file reads.The same test request file is read three times across different fuzz functions. While this works correctly, you could consider reading it once and sharing it if these tests run frequently.
However, since fuzz tests are typically run separately and the current approach ensures test isolation, the existing implementation is perfectly acceptable.
Also applies to: 97-97, 167-167
third_party/httpsig-go/sign_test.go (1)
45-45: Fix typo in test case name.There's a typo in the test case name: "NoMultiValueSuport" should be "NoMultiValueSupport".
- Name: "NoMultiValueSuport", + Name: "NoMultiValueSupport",third_party/httpsig-go/verify_test.go (1)
166-201: Consider consistent error message formatting.The error messages in the metadata provider methods use different formats. Some say "No X value" while the pattern could be more consistent.
- return 0, fmt.Errorf("No created value") + return 0, fmt.Errorf("no created value")- return 0, fmt.Errorf("No expires value") + return 0, fmt.Errorf("no expires value")- return "", fmt.Errorf("No nonce value") + return "", fmt.Errorf("no nonce value")- return "", fmt.Errorf("No alg value") + return "", fmt.Errorf("no alg value")- return "", fmt.Errorf("No keyid value") + return "", fmt.Errorf("no keyid value")- return "", fmt.Errorf("No tag value") + return "", fmt.Errorf("no tag value")third_party/httpsig-go/signatures.go (1)
79-135: Consider consistent error handling throughout the sign function.Some errors use
newErrorwith error codes while others usefmt.Errorf. For better error handling and debugging, consider usingnewErrorconsistently.For example, these errors could be updated:
- return fmt.Errorf("Invalid private key. Requires *rsa.PrivateKey or crypto.Signer: %T", sp.PrivateKey) + return newError(ErrSigSecretKey, fmt.Sprintf("Invalid private key. Requires *rsa.PrivateKey or crypto.Signer: %T", sp.PrivateKey))- return fmt.Errorf("Invalid private key. Requires *ecdsa.PrivateKey or crypto.Signer: %T", sp.PrivateKey) + return newError(ErrSigSecretKey, fmt.Sprintf("Invalid private key. Requires *ecdsa.PrivateKey or crypto.Signer: %T", sp.PrivateKey))third_party/httpsig-go/base.go (3)
50-55: Consider preserving request context for responses.When processing responses, returning
context.Background()loses any context from the original request. This could affect cancellation, timeouts, or context values that might be needed during signature verification.Consider storing and returning the original request context if available.
182-184: Track the multi-value header support limitation.The TODO indicates that headers with multiple values aren't yet supported. This could be a limitation for applications using headers like
AcceptorX-Forwarded-For.Would you like me to open an issue to track implementing support for multi-value headers?
228-231: Empty query parameters need proper handling.The TODO indicates uncertainty about handling empty query parameters. In HTTP,
?param=(empty value) is different from?param(no value) and both are valid. The current implementation rejects empty values.Consider supporting empty query parameter values by checking if the parameter exists in the query map rather than checking if the value is empty.
third_party/httpsig-go/sign.go (1)
313-320: Simplify error checking in nonce generation.When
rand.Readreturns an error, the value ofnis unreliable. The error check should be sufficient.n, err := rand.Read(nonce) - if err != nil || n < 32 { + if err != nil { return "", fmt.Errorf("could not generate nonce") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
third_party/httpsig-go/go.sumis excluded by!**/*.sumthird_party/httpsig-go/testdata/key-rsa-v15.pubis excluded by!**/*.pubthird_party/httpsig-go/testdata/test-key-ecc-p256.pubis excluded by!**/*.pubthird_party/httpsig-go/testdata/test-key-ecc-p384.pubis excluded by!**/*.pubthird_party/httpsig-go/testdata/test-key-ed25519.pubis excluded by!**/*.pubthird_party/httpsig-go/testdata/test-key-rsa-pss.pubis excluded by!**/*.pubthird_party/httpsig-go/testdata/test-key-rsa.pubis excluded by!**/*.pub
📒 Files selected for processing (68)
third_party/httpsig-go/.github/workflows/codeql.yml(1 hunks)third_party/httpsig-go/.github/workflows/wf.yml(1 hunks)third_party/httpsig-go/.semgrepignore(1 hunks)third_party/httpsig-go/.sideignore(1 hunks)third_party/httpsig-go/LICENSE(1 hunks)third_party/httpsig-go/README.md(1 hunks)third_party/httpsig-go/UPDATE_INSTRUCTIONS(1 hunks)third_party/httpsig-go/UPSTREAM_COMMIT(1 hunks)third_party/httpsig-go/accept.go(1 hunks)third_party/httpsig-go/accept_test.go(1 hunks)third_party/httpsig-go/base.go(1 hunks)third_party/httpsig-go/digest.go(1 hunks)third_party/httpsig-go/digest_test.go(1 hunks)third_party/httpsig-go/examples_test.go(1 hunks)third_party/httpsig-go/fz_test.go(1 hunks)third_party/httpsig-go/go.mod(1 hunks)third_party/httpsig-go/http.go(1 hunks)third_party/httpsig-go/keyman/keyman.go(1 hunks)third_party/httpsig-go/keyutil/jwk.go(1 hunks)third_party/httpsig-go/keyutil/jwk_test.go(1 hunks)third_party/httpsig-go/keyutil/keyutil.go(1 hunks)third_party/httpsig-go/keyutil/testdata/test-jwk-ec.json(1 hunks)third_party/httpsig-go/keyutil/testdata/test-jwk-symmetric.json(1 hunks)third_party/httpsig-go/roundtrip_test.go(1 hunks)third_party/httpsig-go/side.toml(1 hunks)third_party/httpsig-go/sigerrors.go(1 hunks)third_party/httpsig-go/sign.go(1 hunks)third_party/httpsig-go/sign_test.go(1 hunks)third_party/httpsig-go/signatures.go(1 hunks)third_party/httpsig-go/sigtest/helpers.go(1 hunks)third_party/httpsig-go/spec_test.go(1 hunks)third_party/httpsig-go/testdata/b21_request_sigbase.txt(1 hunks)third_party/httpsig-go/testdata/b21_request_signed.txt(1 hunks)third_party/httpsig-go/testdata/b22_request_sigbase.txt(1 hunks)third_party/httpsig-go/testdata/b22_request_signed.txt(1 hunks)third_party/httpsig-go/testdata/b23_request_sigbase.txt(1 hunks)third_party/httpsig-go/testdata/b23_request_signed.txt(1 hunks)third_party/httpsig-go/testdata/b24_response_sigbase.txt(1 hunks)third_party/httpsig-go/testdata/b24_response_signed.txt(1 hunks)third_party/httpsig-go/testdata/b25_request_sigbase.txt(1 hunks)third_party/httpsig-go/testdata/b25_request_signed.txt(1 hunks)third_party/httpsig-go/testdata/b26_request_sigbase.txt(1 hunks)third_party/httpsig-go/testdata/b26_request_signed.txt(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzExtractSignatures/00e15d22123489fd(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzExtractSignatures/85649d45641911d0(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzSign/01c151dfb31ce1b1(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzSign/2da5aa149625a656(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzSign/58358ab8a686cc26(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzSign/ecf6af09b9da5a62(1 hunks)third_party/httpsig-go/testdata/fuzz/FuzzSign/ff08bb6e0efa59be(1 hunks)third_party/httpsig-go/testdata/key-rsa-v15.key(1 hunks)third_party/httpsig-go/testdata/request1.txt(1 hunks)third_party/httpsig-go/testdata/request_bad_digest.txt(1 hunks)third_party/httpsig-go/testdata/request_multivalue.txt(1 hunks)third_party/httpsig-go/testdata/request_no_digest.txt(1 hunks)third_party/httpsig-go/testdata/request_repeated_components.txt(1 hunks)third_party/httpsig-go/testdata/rfc-test-request.txt(1 hunks)third_party/httpsig-go/testdata/rfc-test-response.txt(1 hunks)third_party/httpsig-go/testdata/test-key-ecc-p256.key(1 hunks)third_party/httpsig-go/testdata/test-key-ecc-p384.key(1 hunks)third_party/httpsig-go/testdata/test-key-ed25519.key(1 hunks)third_party/httpsig-go/testdata/test-key-rsa-pss.key(1 hunks)third_party/httpsig-go/testdata/test-key-rsa.key(1 hunks)third_party/httpsig-go/testdata/test-shared-secret(1 hunks)third_party/httpsig-go/testdata/verify_request1.txt(1 hunks)third_party/httpsig-go/testdata/verify_request2.txt(1 hunks)third_party/httpsig-go/verify.go(1 hunks)third_party/httpsig-go/verify_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
third_party/httpsig-go/testdata/test-shared-secret (1)
Learnt from: getvictor
PR: fleetdm/fleet#30589
File: ee/server/service/hostidentity/depot/depot.go:104-119
Timestamp: 2025-07-08T16:06:54.576Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow certificate revocation (lines 104-119) is a known issue that will be addressed in a later feature, not an immediate concern to fix.
third_party/httpsig-go/go.mod (1)
Learnt from: lucasmrod
PR: fleetdm/fleet#30550
File: server/datastore/mysql/policies_test.go:6368-6378
Timestamp: 2025-07-03T15:13:36.494Z
Learning: Go 1.22 introduced a significant change to loop variable semantics where each iteration of a for loop creates a new variable, automatically fixing the classic closure capture bug that existed in earlier Go versions. Code that captures loop variables in closures no longer needs explicit variable copying in Go 1.22+. This fix is only enabled if the module's go.mod file specifies go 1.22 or higher.
🧬 Code Graph Analysis (8)
third_party/httpsig-go/accept.go (2)
third_party/httpsig-go/sign.go (8)
SigningProfile(55-63)MetaNonce(41-41)MetaKeyID(43-43)MetaTag(44-44)Fields(73-82)Metadata(20-20)MetaAlgorithm(42-42)Algorithm(16-16)third_party/httpsig-go/sigerrors.go (2)
ErrInvalidAcceptSignature(37-37)ErrMissingAcceptSignature(38-38)
third_party/httpsig-go/keyman/keyman.go (2)
third_party/httpsig-go/verify.go (2)
KeySpec(41-46)KeySpecer(54-56)third_party/httpsig-go/signatures.go (1)
MetadataProvider(30-37)
third_party/httpsig-go/sigtest/helpers.go (2)
third_party/httpsig-go/keyutil/keyutil.go (2)
ReadPublicKey(39-60)ReadPrivateKey(89-122)third_party/httpsig-go/keyutil/jwk_test.go (1)
Diff(136-142)
third_party/httpsig-go/keyutil/jwk_test.go (2)
third_party/httpsig-go/keyutil/jwk.go (4)
JWK(93-99)ReadJWKFile(19-25)ReadJWK(27-58)FromPrivateKey(70-90)third_party/httpsig-go/sigtest/helpers.go (1)
Diff(79-85)
third_party/httpsig-go/digest.go (2)
third_party/httpsig-go/sign.go (3)
Digest(17-17)DigestSHA256(35-35)DigestSHA512(36-36)third_party/httpsig-go/sigerrors.go (4)
ErrNoSigUnsupportedDigest(16-16)ErrNoSigMessageBody(20-20)ErrInternal(41-41)ErrNoSigInvalidHeader(15-15)
third_party/httpsig-go/sign_test.go (4)
third_party/httpsig-go/sigerrors.go (5)
ErrCode(9-9)ErrInvalidSignatureOptions(32-32)ErrInvalidComponent(33-33)ErrUnsupported(42-42)ErrInvalidMetadata(34-34)third_party/httpsig-go/sign.go (8)
Metadata(20-20)MetaCreated(39-39)MetaExpires(40-40)MetaNonce(41-41)MetaAlgorithm(42-42)MetaKeyID(43-43)MetaTag(44-44)SignedField(67-70)third_party/httpsig-go/sigtest/helpers.go (2)
ReadRequest(18-29)Diff(79-85)third_party/httpsig-go/keyutil/jwk_test.go (1)
Diff(136-142)
third_party/httpsig-go/sigerrors.go (1)
server/fleet/errors.go (2)
Cause(556-564)Error(435-440)
third_party/httpsig-go/sign.go (2)
server/fleet/app.go (1)
Duration(835-837)third_party/httpsig-go/sigerrors.go (1)
ErrInvalidSignatureOptions(32-32)
🪛 Gitleaks (8.26.0)
third_party/httpsig-go/testdata/test-key-ecc-p384.key
1-6: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
third_party/httpsig-go/testdata/test-key-rsa-pss.key
1-1: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
third_party/httpsig-go/testdata/test-key-ecc-p256.key
1-6: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
third_party/httpsig-go/testdata/test-key-ed25519.key
1-1: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
third_party/httpsig-go/examples_test.go
16-20: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🪛 LanguageTool
third_party/httpsig-go/README.md
[grammar] ~10-~10: Ensure spelling is correct
Context: ...ble to) used for demonstrating proof-of-posession ([DPoP](https://www.rfc-editor.org/rfc/...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~13-~13: ‘with the exception of’ might be wordy. Consider a shorter alternative.
Context: ...res The full specification is supported with the exception of the following. File a ticket or PR and ...
(EN_WORDINESS_PREMIUM_WITH_THE_EXCEPTION_OF)
🪛 markdownlint-cli2 (0.17.2)
third_party/httpsig-go/README.md
21-21: Hard tabs
Column: 1
(MD010, no-hard-tabs)
22-22: Hard tabs
Column: 1
(MD010, no-hard-tabs)
23-23: Hard tabs
Column: 1
(MD010, no-hard-tabs)
24-24: Hard tabs
Column: 1
(MD010, no-hard-tabs)
25-25: Hard tabs
Column: 1
(MD010, no-hard-tabs)
26-26: Hard tabs
Column: 1
(MD010, no-hard-tabs)
27-27: Hard tabs
Column: 1
(MD010, no-hard-tabs)
29-29: Hard tabs
Column: 1
(MD010, no-hard-tabs)
30-30: Hard tabs
Column: 1
(MD010, no-hard-tabs)
32-32: Hard tabs
Column: 1
(MD010, no-hard-tabs)
33-33: Hard tabs
Column: 1
(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1
(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1
(MD010, no-hard-tabs)
40-40: Hard tabs
Column: 1
(MD010, no-hard-tabs)
41-41: Hard tabs
Column: 1
(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
43-43: Hard tabs
Column: 1
(MD010, no-hard-tabs)
44-44: Hard tabs
Column: 1
(MD010, no-hard-tabs)
45-45: Hard tabs
Column: 1
(MD010, no-hard-tabs)
46-46: Hard tabs
Column: 1
(MD010, no-hard-tabs)
48-48: Hard tabs
Column: 1
(MD010, no-hard-tabs)
49-49: Hard tabs
Column: 1
(MD010, no-hard-tabs)
51-51: Hard tabs
Column: 1
(MD010, no-hard-tabs)
52-52: Hard tabs
Column: 1
(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (58)
third_party/httpsig-go/.semgrepignore (1)
1-1: scope is tight enough — good jobThe pattern only skips
testdata/*.key, so other files still get scanned. That feels like the right balance between convenience and safety.third_party/httpsig-go/testdata/test-shared-secret (1)
1-1: double-check the origin of this secretEven though it is only for tests, please confirm the string is synthetic (not copied from prod) and documented as such. A short comment in the file or in the test that consumes it would help future readers.
third_party/httpsig-go/testdata/b21_request_sigbase.txt (1)
1-1: looks goodThe vector matches the spec format. No action needed.
third_party/httpsig-go/LICENSE (1)
1-21: license included — thank youIncluding the full MIT license keeps us compliant. Nothing to change here.
third_party/httpsig-go/testdata/rfc-test-request.txt (1)
1-9: check license headers for vendored contentbecause these files live in
third_party/, fleet must ship the upstream license that covers the httpsig-go test vectors. please verify that the corresponding license file is present at the root of the vendored tree so downstream users stay in compliance.third_party/httpsig-go/testdata/request_repeated_components.txt (1)
1-11: looks good — valuable edge-case vectorthe repeated header values are helpful for spec coverage. no issues spotted.
third_party/httpsig-go/testdata/request_multivalue.txt (1)
1-10: multi-value header test case is cleareverything checks out and will aid signature-base calculations.
third_party/httpsig-go/.github/workflows/codeql.yml (1)
30-32: Verify the conditional runner expressionThe ternary-style expression is fine, but if you ever add more languages it will still default to
ubuntu-latest. Confirm that’s intended.third_party/httpsig-go/testdata/fuzz/FuzzSign/ecf6af09b9da5a62 (1)
1-4: No issues – seed corpus looks reasonableThe seeds are minimal and will be deduplicated by
go test fuzz.third_party/httpsig-go/side.toml (1)
1-2: Confirm zero-iteration settings
max_iterations = 0andmax_planning_iterations = 0disable Side’s reasoning loops. Double-check this is intentional; otherwise set sensible limits (e.g., 3).third_party/httpsig-go/UPSTREAM_COMMIT (1)
1-1: Upstream commit marker looks goodThe hash is recorded and a trailing newline is present. No issues spotted.
third_party/httpsig-go/testdata/test-key-ecc-p384.key (1)
1-6: Test-only private key detected – confirm it stays test-onlyThe PEM block is fine for unit tests, but ensure:
- It is not packaged in production binaries or Docker images.
- Secret-scanning rules are updated to ignore this specific test path to avoid future pipeline noise.
If either point is unclear, let’s double-check the build scripts.
third_party/httpsig-go/testdata/key-rsa-v15.key (1)
1-17: private keys in repo: double-check test-only scope and packagingthis pem block is useful for unit tests, but shipping raw private keys inside the public repo can trigger scanners and inflate docker images. please confirm that:
- the
third_party/httpsig-go/testdata/tree is excluded from any production builds, docker images, and release archives..semgrepignore(or similar) covers these paths so ci won’t fail on secret-detection gates.- licence of the vendored library permits redistributing these keys.
no functional change requested if the above is already true.
third_party/httpsig-go/testdata/request_no_digest.txt (1)
1-8: looks good – negative-case vector is usefulincluding a request without a
Digestheader exercises important edge-case logic indigest.go. nice addition.third_party/httpsig-go/testdata/b22_request_sigbase.txt (1)
1-4: looks good – canonical vector preservedthe signature-base matches rfc 9421 §3.3 example B-22. no issues spotted.
third_party/httpsig-go/testdata/b23_request_sigbase.txt (1)
1-9: vector aligns with spec – no change neededheader order and params mirror rfc 9421 example B-23. all good.
third_party/httpsig-go/testdata/request_bad_digest.txt (1)
1-8: good negative test caseincluding a deliberately wrong
Content-Digestis valuable for verifier tests.third_party/httpsig-go/testdata/b24_response_sigbase.txt (1)
1-5: looks good, base string matches rfc 9421I spot-checked the
content-length(23) against the body in the companion signed response and they line up, so this vector is internally consistent.third_party/httpsig-go/testdata/b26_request_sigbase.txt (1)
1-7: request vector looks consistentContent-length and header order follow the spec. No concerns here.
third_party/httpsig-go/testdata/b21_request_signed.txt (1)
1-10: Digest verified and empty header list is validI used Python to compute the SHA-512 digest of the body and it matches the Content-Digest header exactly. An empty header list
()in Signature-Input is uncommon but perfectly valid per the HTTP Message Signatures spec. If this test is intended to cover signing with no headers, no changes are needed.third_party/httpsig-go/testdata/b24_response_signed.txt (1)
1-9: signed response looks solidThe status line, headers, and body length line up, and the signature input appears correct. Nice work.
third_party/httpsig-go/testdata/verify_request2.txt (1)
1-11: Looks good.Valid test vector; no issues spotted.
third_party/httpsig-go/testdata/b22_request_signed.txt (1)
1-11: All set.Signature headers and payload line up nicely.
third_party/httpsig-go/testdata/b25_request_signed.txt (1)
1-11: Good to go.The signed request provides useful coverage for shared-secret algorithms.
third_party/httpsig-go/testdata/b26_request_signed.txt (1)
1-11: LGTM.Ed25519 vector looks correct and will help keep the tests robust.
third_party/httpsig-go/testdata/b23_request_signed.txt (1)
1-11: Test data looks good for HTTP signature validation.This test file provides a complete example of an HTTP request with proper signature headers, which will be valuable for testing the signature verification functionality.
third_party/httpsig-go/accept_test.go (1)
9-64: Well-structured test coverage for Accept-Signature parsing.The test cases provide comprehensive coverage of both valid and invalid scenarios, following Go testing best practices with table-driven tests. The error code validation ensures robust error handling.
third_party/httpsig-go/keyman/keyman.go (1)
17-22: Good defensive programming with nil map handling.The constructor properly initializes an empty map when nil is passed, preventing potential panic conditions.
third_party/httpsig-go/UPDATE_INSTRUCTIONS (2)
4-6: Please update the placeholder path and test these instructions.The instructions contain a placeholder path and note they haven't been fully tested. Consider updating the path to match your actual repository structure and validating the process works correctly.
8-42: Well-structured process for maintaining vendored dependencies.The git workflow outlined here follows good practices for maintaining vendored code while preserving downstream modifications. The use of separate branches and rsync for file operations is appropriate.
third_party/httpsig-go/accept.go (2)
16-68: Robust Accept-Signature header parsing implementation.The parsing logic properly handles the structured field value format, validates input types, and extracts all relevant metadata. Error handling is comprehensive and follows the established patterns in the library.
27-32: Good validation of dictionary structure requirements.The code properly validates that Accept-Signature values must be dictionaries with inner lists, providing clear error messages when the format is incorrect.
third_party/httpsig-go/sigtest/helpers.go (1)
1-85: The helper functions are well-structured and follow good testing practices.This test utility package provides clean, reusable functions for loading test data and comparing results. The implementation correctly handles file I/O errors and integrates well with Go's testing framework.
third_party/httpsig-go/examples_test.go (4)
15-37: Good example demonstrating basic signing workflow.This example clearly shows how to create a signer with ECDSA P-256 and sign an HTTP request. The code demonstrates proper key loading and signing profile configuration.
16-20: The embedded private key in examples is expected and safe.The static analysis tool flagged this private key, but this is completely appropriate for example/documentation code. The key is clearly intended for demonstration purposes and helps users understand the expected format.
39-76: Excellent examples showing verification and handler integration.These examples effectively demonstrate how to verify signatures and integrate with HTTP handlers. The pattern of checking verification results in the context is particularly helpful for developers.
78-104: Clear demonstration of HTTP client integration.This example shows both signing-only and sign-and-verify HTTP clients, which covers the main client-side use cases. The API design looks intuitive and follows Go conventions.
third_party/httpsig-go/keyutil/jwk_test.go (2)
16-74: Comprehensive test coverage for JWK parsing functionality.The test cases cover both valid inputs (EC and symmetric JWKs) and error cases (malformed JSON, empty input). The use of both file-based and string-based inputs provides good test coverage.
76-134: Good roundtrip testing ensures data integrity.The roundtrip test validates that marshaling and unmarshaling preserves all data correctly. This is particularly important for cryptographic key material where any data loss could cause security issues.
third_party/httpsig-go/digest.go (4)
14-22: Efficient use of constants and pre-computed empty digests.The pre-computed empty digest values are a nice optimization that avoids redundant calculations for empty request bodies. The constants follow Go naming conventions.
30-71: Robust digest calculation with proper body handling.The function correctly handles different body states (nil, NoBody, actual content) and properly reconstructs the body for continued use. The error handling is comprehensive and uses appropriate error types.
73-90: Proper structured field value formatting for digest headers.The use of the
httpsfvlibrary for creating properly formatted digest headers follows RFC 8941 standards. The error handling covers both algorithm validation and marshaling failures.
92-123: Good header parsing with graceful handling of unsupported algorithms.The function correctly parses the Content-Digest header and gracefully skips unsupported or malformed entries while returning the first supported digest found. This approach provides good compatibility with various implementations.
third_party/httpsig-go/roundtrip_test.go (3)
13-168: Excellent comprehensive test coverage across all supported algorithms.The test suite covers RSA-PSS, RSA-v1.5, HMAC-SHA256, ECDSA (P-256 and P-384), and Ed25519 algorithms. The inclusion of both success and failure cases (like BadDigest) provides thorough validation of the signing and verification workflow.
170-211: Well-structured test execution with proper error handling.The test loop properly handles both expected successes and expected failures, with detailed logging for debugging. The verification of error codes ensures that failures happen for the right reasons.
213-217: Clean helper function for creating verification profiles.This utility function nicely encapsulates the common pattern of creating verification profiles with custom labels, making the test cases more readable.
third_party/httpsig-go/digest_test.go (1)
12-63: Test coverage looks comprehensive.The digest creation tests cover both supported algorithms (SHA-256, SHA-512) and error handling for unsupported algorithms. The test structure is well-organized and follows good practices.
third_party/httpsig-go/fz_test.go (1)
13-79: Fuzz test implementation looks solid.The fuzz tests comprehensively cover signing options, fields validation, and signature extraction. The error handling appropriately distinguishes between expected
SignatureErrortypes and unexpected errors, which is excellent for robustness testing.Also applies to: 81-146, 148-200
third_party/httpsig-go/keyutil/keyutil.go (2)
124-143: RSA-PSS key handling implementation is well-designed.The custom RSA-PSS parsing logic correctly handles the limitation in Go 1.19's PKCS#8 parser. The ASN.1 unmarshaling approach and OID verification provide a robust workaround for unsupported RSA-PSS keys.
20-27: "Must" functions follow Go conventions.The panic-on-error variants are appropriately named and implemented, following established Go patterns for convenience functions in test and initialization code.
Also applies to: 62-69, 71-77
third_party/httpsig-go/http.go (2)
44-47: Good defensive programming with nil check.The panic on nil verifier is appropriate here since the function contract requires a non-nil verifier for proper operation. This prevents runtime errors down the line.
84-102: Transport implementation handles signing and verification correctly.The RoundTrip method properly sequences signing before the request and verification after the response, with appropriate error handling for both operations.
third_party/httpsig-go/sign_test.go (2)
23-127: Comprehensive test coverage for signature base calculation.The test cases thoroughly cover various error conditions including repeated components, invalid component names, unsupported features, and metadata errors. The table-driven approach makes the tests clear and maintainable.
183-257: Mock metadata providers are well-implemented.The
errorMetadataProviderandfixedMetadataProvidertypes provide good test doubles for validating error handling and successful metadata extraction scenarios.third_party/httpsig-go/sigerrors.go (1)
1-89: Well-structured error handling implementation!The error code system is thoughtfully organized into categories, and the
SignatureErrortype properly implements the error interface with unwrap support for error chaining. This follows Go's error handling best practices nicely.third_party/httpsig-go/spec_test.go (1)
1-385: Excellent spec compliance testing!The tests thoroughly validate the implementation against RFC 9421 test vectors, covering signature verification, base string generation, and deterministic signature recreation. The table-driven approach makes it easy to add new test cases.
third_party/httpsig-go/sign.go (1)
190-196: Add explicit handling for all nonce schemes.The switch statement only handles
NonceRandom32but doesn't have a default case. The error return after the switch might not be reachable in all code paths.func (s *Signer) Nonce() (string, error) { switch s.profile.Nonce { case NonceRandom32: return nonceRandom32() + default: + return "", fmt.Errorf("Invalid nonce scheme '%d'", s.profile.Nonce) } - return "", fmt.Errorf("Invalid nonce scheme '%d'", s.profile.Nonce) }Likely an incorrect or invalid review comment.
third_party/httpsig-go/verify.go (1)
366-376: Add missing return statement for HMAC verification.The HMAC verification case is missing a return statement after successful verification, causing it to fall through to the default case.
case Algo_HMAC_SHA256: if len(ks.Secret) == 0 { return specer, newError(ErrInvalidSignatureOptions, fmt.Sprintf("No secret provided for symmetric algorithm '%s'", Algo_HMAC_SHA256)) } msgHash := hmac.New(sha256.New, ks.Secret) msgHash.Write(base.base) // write does not return an error per hash.Hash documentation calcualtedSignature := msgHash.Sum(nil) if !hmac.Equal(calcualtedSignature, sig.Signature) { return specer, newError(ErrSigVerification, fmt.Sprintf("Signature did not verify for algo '%s'", ks.Algo), err) } + return specer, nil⛔ Skipped due to learnings
Learnt from: getvictor PR: fleetdm/fleet#30589 File: ee/server/service/hostidentity/depot/depot.go:104-119 Timestamp: 2025-07-08T16:06:54.576Z Learning: In ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow certificate revocation (lines 104-119) is a known issue that will be addressed in a later feature, not an immediate concern to fix.
lukeheath
left a comment
lukeheath
left a comment
There was a problem hiding this comment.
Approving the new third party repo as well as this new organization structure for adding and updating third party repos.
@getvictor will follow up later with an ADR to document this decision so we can share with the team and standardize this process. Thanks!
3 participants
For #30473
This change adds a vendored
httpsig-golibrary to our repo. We cannot use the upstream library because it has not merged the change we need: remitly-oss/httpsig-go#25Thus, we need our own copy at this point.
The instructions for keeping this library up to date (if needed) are in
UPDATE_INSTRUCTIONS.None of the coderabbitai review comments are relevant to the code/features we are going to use for HTTP message signatures.
We will use this library in subsequent PRs for the TPM-backed HTTP message signature feature.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores