Security: Sensitive Information Exposure in TunnelInfo Model#776
Security: Sensitive Information Exposure in TunnelInfo Model#776tomaioo wants to merge 1 commit into
Conversation
The `TunnelInfo` Pydantic model stores sensitive credentials including `frp_token`, `binding_secret`, and `http_password` as plain string fields. These credentials are exposed in the model without any encryption or masking. Additionally, the model includes a comment indicating that `http_password` is 'never retrievable afterwards' but it is still stored as a plain string field without any special handling. Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
tomaioo
requested review from
JannikSt,
JohannesHa,
burnpiro,
d42me,
kcoopermiller and
willccbb
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 01fa645. Configure here.
| frp_token: str = Field(..., description="Authentication token for frpc") | ||
| binding_secret: str = Field("", description="Per-tunnel secret for frpc metadata") | ||
| frp_token: SecretStr = Field(..., description="Authentication token for frpc") | ||
| binding_secret: SecretStr = Field(default=SecretStr(""), description="Per-tunnel secret for frpc metadata") |
There was a problem hiding this comment.
SecretStr masked in frpc config
High Severity
TunnelInfo now types frp_token and binding_secret as SecretStr, but _write_frpc_config still embeds them in f-strings. Pydantic masks SecretStr in string conversion, so the generated frpc config gets placeholder values instead of the real token and binding secret, and the client cannot authenticate to frps.
Reviewed by Cursor Bugbot for commit 01fa645. Configure here.
| None, description="HTTP basic auth username, if auth is enabled" | ||
| ) | ||
| http_password: Optional[str] = Field( | ||
| http_password: Optional[SecretStr] = Field( |
There was a problem hiding this comment.
http_password returns SecretStr not str
Medium Severity
The Tunnel.http_password property is annotated as Optional[str] but forwards TunnelInfo.http_password, which is now Optional[SecretStr]. Callers that expect a plain string for HTTP basic auth or logging get a SecretStr instance instead of the password text unless they call get_secret_value() themselves.
Reviewed by Cursor Bugbot for commit 01fa645. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01fa64545d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| frp_token: SecretStr = Field(..., description="Authentication token for frpc") | ||
| binding_secret: SecretStr = Field(default=SecretStr(""), description="Per-tunnel secret for frpc metadata") |
There was a problem hiding this comment.
With these fields now typed as SecretStr, f-stringing self._tunnel_info.frp_token and binding_secret in Tunnel._write_frpc_config() no longer inserts the backend values; SecretStr renders as masked text. Every Tunnel.start() will write masked credentials such as auth.token = "**********" into the frpc config, so frpc authenticates with the wrong token and new tunnels fail to connect. The config path needs to use get_secret_value() or another explicit unwrapping step before formatting.
Useful? React with 👍 / 👎.
| None, description="HTTP basic auth username, if auth is enabled" | ||
| ) | ||
| http_password: Optional[str] = Field( | ||
| http_password: Optional[SecretStr] = Field( |
There was a problem hiding this comment.
When http_user is set, the backend returns http_password only in the create response and the CLI prints tunnel.http_password to the user. After changing this field to SecretStr, that property returns a secret object whose string representation is masked, so the CLI displays ********** and the user never receives the password needed to access the tunnel. Keep the model secret internally but expose get_secret_value() through the property or CLI.
Useful? React with 👍 / 👎.
1 participant
Summary
Security: Sensitive Information Exposure in TunnelInfo Model
Problem
Severity:
High| File:packages/prime-tunnel/src/prime_tunnel/models.py:L1The
TunnelInfoPydantic model stores sensitive credentials includingfrp_token,binding_secret, andhttp_passwordas plain string fields. These credentials are exposed in the model without any encryption or masking. Additionally, the model includes a comment indicating thathttp_passwordis 'never retrievable afterwards' but it is still stored as a plain string field without any special handling.Solution
Implement secure handling for sensitive fields using Pydantic's
SecretStrtype to prevent accidental logging or serialization of credentials. Add proper access controls and consider encrypting these values at rest.Changes
packages/prime-tunnel/src/prime_tunnel/models.py(modified)Note
Medium Risk
Credential fields change type, so any code that expects plain strings (e.g. f-string config generation) must use
.get_secret_value()or serialization may break or leak masked placeholders.Overview
TunnelInfonow treatsfrp_token,binding_secret, and optionalhttp_passwordas PydanticSecretStrinstead of plain strings, so default model dumps and logs mask secrets instead of exposing raw credentials.binding_secretkeeps an empty default viaSecretStr(""); other tunnel metadata fields are unchanged.Reviewed by Cursor Bugbot for commit 01fa645. Bugbot is set up for automated code reviews on this repo. Configure here.