Skip to content

starknet_transaction_prover: add TLS integration tests#14050

Open
avi-starkware wants to merge 1 commit into
avi/privacy/config-validation-tests-v2from
avi/privacy/tls-tests-v2
Open

starknet_transaction_prover: add TLS integration tests#14050
avi-starkware wants to merge 1 commit into
avi/privacy/config-validation-tests-v2from
avi/privacy/tls-tests-v2

Conversation

@avi-starkware

Copy link
Copy Markdown
Collaborator

Adds end-to-end HTTPS tests against a self-signed test certificate
checked into resources/test_tls/: a successful HTTPS JSON-RPC roundtrip
on starknet_specVersion, a plain-HTTP-against-TLS-server negative test,
and unit tests around load_tls_acceptor covering missing cert/key
paths, invalid PEM contents, and the happy path.

To allow co-located tests in tls_test.rs to call it, load_tls_acceptor
is bumped from private to pub(crate); no API surface is exposed
outside the crate.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@cursor

cursor Bot commented May 17, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only code and fixtures; the only production change is widening load_tls_acceptor visibility within the crate, with no external API impact.

Overview
Adds TLS integration and unit tests for starknet_transaction_prover without changing production HTTPS behavior.

New resources/test_tls/ holds a documented localhost self-signed cert/key fixture for tests only. tls_test.rs boots start_tls_server with that material and checks a successful HTTPS starknet_specVersion JSON-RPC roundtrip (reqwest trusts the fixture cert), plus a negative case where plain HTTP against the TLS listener fails.

rstest parametrizes load_tls_acceptor failures (missing cert/key, invalid PEM) and a success path with the crypto provider installed for nextest. load_tls_acceptor is pub(crate) so tests can call it; server.rs registers the tls_test module under #[cfg(test)].

Reviewed by Cursor Bugbot for commit 02903ed. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment on lines +1 to +2
-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCcDEcAhUbdyBnU

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Semgrep identified an issue in your code:

Hard-coded private key in source code allows attackers who gain repository access to impersonate your service or decrypt sensitive data.

More details about this

This code contains a hard-coded private key embedded directly in your source code. An attacker who gains access to your repository—whether through a compromised developer account, leaked source code, or cloned Git history—would immediately obtain this private key.

Here's a concrete attack scenario:

  1. Attacker gains repository access: A developer's GitHub account is compromised, or the repo is accidentally made public, exposing the code containing the -----BEGIN PRIVATE KEY----- block.

  2. Attacker extracts the key: The attacker copies the entire private key (the base64-encoded $KEY content between the BEGIN/END markers).

  3. Attacker impersonates your service: Using this private key, the attacker can:

    • Sign JWTs or other tokens to forge authentication credentials
    • Decrypt data encrypted with the corresponding public key
    • Establish TLS connections pretending to be your service
    • Access any system that trusts this key for authentication
  4. Lateral movement: If this key is used to authenticate to cloud services, databases, or internal APIs, the attacker now has a foothold into your infrastructure.

The danger is that this exposure is permanent—even if you delete the key from your repo now, it remains in Git history and anyone with repository access can retrieve it.

To resolve this comment:

✨ Commit fix suggestion
  1. Remove the entire private key block from your codebase, including everything from -----BEGIN PRIVATE KEY----- to -----END PRIVATE KEY-----.
  2. Store the private key in a secure location outside your source code, such as a private file on the server, a secure secrets manager, or environment variable.
  3. Update your application to load the private key at runtime from the secure location instead of from the code. For example, use fs.readFileSync('/path/to/private.key') in Node.js, or reference an environment variable that points to the file path.
  4. Add the filename or pattern for private keys (such as *.key or the specific key file) to your .gitignore file to prevent accidental commits in the future.
💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by detected-private-key.

You can view more details about this finding in the Semgrep AppSec Platform.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

/fp Test fixture only. Cert subject is CN=localhost (SAN: DNS:localhost,IP:127.0.0.1); used exclusively by src/server/tls_test.rs to exercise start_tls_server and load_tls_acceptor. Provenance and regeneration command documented in resources/test_tls/README.md. The key is not referenced from any production code path.

Comment thread crates/starknet_transaction_prover/src/server/tls_test.rs
@avi-starkware avi-starkware force-pushed the avi/privacy/config-validation-tests-v2 branch from 3bf5954 to 8600ded Compare May 17, 2026 17:39
@avi-starkware avi-starkware force-pushed the avi/privacy/tls-tests-v2 branch from b062874 to 1d6f06e Compare May 17, 2026 17:39
@avi-starkware avi-starkware force-pushed the avi/privacy/config-validation-tests-v2 branch from 8600ded to 96bf8d9 Compare May 17, 2026 19:07
@avi-starkware avi-starkware force-pushed the avi/privacy/tls-tests-v2 branch 2 times, most recently from 6377a5b to 6a4087a Compare May 20, 2026 08:33
@avi-starkware avi-starkware force-pushed the avi/privacy/config-validation-tests-v2 branch from 96bf8d9 to 056f406 Compare May 20, 2026 08:33
@avi-starkware avi-starkware force-pushed the avi/privacy/config-validation-tests-v2 branch from 056f406 to 3a5608b Compare May 31, 2026 10:39
@avi-starkware avi-starkware force-pushed the avi/privacy/tls-tests-v2 branch from 6a4087a to 44827d2 Compare May 31, 2026 10:39
Adds end-to-end HTTPS tests against a self-signed test certificate
checked into resources/test_tls/: a successful HTTPS JSON-RPC roundtrip
on starknet_specVersion, a plain-HTTP-against-TLS-server negative test,
and unit tests around load_tls_acceptor covering missing cert/key
paths, invalid PEM contents, and the happy path.

To allow co-located tests in tls_test.rs to call it, load_tls_acceptor
is bumped from private to pub(crate); no API surface is exposed
outside the crate.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 02903ed. Configure here.


let response = client
.post(format!("https://localhost:{}", addr.port()))
.json(&spec_version_request())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Client URL mismatches bind address

Medium Severity

The TLS server is bound to 127.0.0.1, but the integration tests post to https://localhost:… and http://localhost:…. On hosts where localhost resolves to ::1 first, nothing listens on that address, so the HTTPS case can fail flakily and the HTTP negative case may error with connection refused instead of exercising plain HTTP against a TLS listener.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 02903ed. Configure here.

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