Skip to content

Conversation

jclapis
Copy link
Collaborator

@jclapis jclapis commented May 21, 2025

The signer module currently hardcodes the IP it binds to as 0.0.0.0, and the port isn't able to be specified in the configuration file - it's only pulled from an environment variable. In Docker mode it doesn't matter, but that's not ideal for people running natively that want to restrict the traffic to localhost since it will open up the Signer API to the entire network.

This modifies it to match PBS's setup, where host (IP) and port are both configuration variables. Users can manually override the combo with the CB_SIGNER_ENDPOINT environment variable which is an analog of the PBS one. It also specifies a default signer host (localhost) and port (20000) in case the configuration parameters are unset and the environment variable isn't present.

@jclapis jclapis requested review from ManuelBilbao and ltitanb May 21, 2025 16:31
@jclapis jclapis self-assigned this May 21, 2025
@jclapis jclapis added the signer Signer module label May 21, 2025
@jclapis jclapis force-pushed the add-ip-bind-to-signer branch from 2e2ba3b to c0f591d Compare May 28, 2025 04:59
@jclapis jclapis marked this pull request as ready for review June 3, 2025 05:00
Copy link
Collaborator

@ltitanb ltitanb left a comment

Choose a reason for hiding this comment

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

looks good overall! some small comments, also could you please update the config.example.toml ?

@jclapis
Copy link
Collaborator Author

jclapis commented Jun 9, 2025

looks good overall! some small comments, also could you please update the config.example.toml ?

Done in 7afb763.

@ltitanb ltitanb self-requested a review June 9, 2025 19:46
@ltitanb ltitanb merged commit a417258 into main Jun 9, 2025
2 checks passed
@ltitanb ltitanb deleted the add-ip-bind-to-signer branch June 9, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
signer Signer module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants