Skip to content

fix(tasks) Add rpc_secret to taskworker#593

Merged
markstory merged 5 commits intomainfrom
fix-taskworker-secret
Mar 31, 2026
Merged

fix(tasks) Add rpc_secret to taskworker#593
markstory merged 5 commits intomainfrom
fix-taskworker-secret

Conversation

@markstory
Copy link
Copy Markdown
Member

In production we need an RPC secret to sign requests to taskbroker. Instead of using the existing shared secret between launchpad and sentry, we should use a different key.

In production we need an RPC secret to sign requests to taskbroker.
Instead of using the existing shared secret between launchpad and
sentry, we should use a different key.
@sentry
Copy link
Copy Markdown
Contributor

sentry Bot commented Mar 30, 2026

Sentry Build Distribution

App Name App ID Version Configuration Install Page
Hacker News com.emergetools.hackernews 1.0.2 (13) Release Install Build

Comment thread src/launchpad/worker/app.py Outdated
Comment on lines +104 to +106
app.set_config({
"rpc_secret": os.getenv("TASKWORKER_SHARED_SECRET"),
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: os.getenv("TASKWORKER_SHARED_SECRET") is called without a default value or validation. If the environment variable is not set, None is passed as the rpc_secret, likely causing runtime failures.
Severity: HIGH

Suggested Fix

Add a validation check to ensure the value from os.getenv("TASKWORKER_SHARED_SECRET") is not None before calling app.set_config(). If the value is missing, raise a RuntimeError to fail fast, similar to the pattern used for LAUNCHPAD_RPC_SHARED_SECRET elsewhere in the codebase.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/launchpad/worker/app.py#L104-L106

Potential issue: The `rpc_secret` configuration is set using
`os.getenv("TASKWORKER_SHARED_SECRET")` without a default value or subsequent
validation. If this environment variable is not set, the function returns `None`, which
is then passed to `app.set_config()`. Since the secret is described as mandatory for
production, passing `None` to the `taskbroker-client` library will likely result in a
runtime `TypeError` or silent authentication failures when the worker attempts to
communicate with the taskbroker. This contrasts with existing patterns in the codebase
where a missing secret raises a `RuntimeError`.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

None is fine.

the launchpad env vars have just a string (for now) so we need to do
some munging to get it into the correct shape for taskbroker-client
this variable comes from deployment manifests and the shared
taskbroker-secrets.
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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, enable autofix in the Cursor dashboard.

Comment thread src/launchpad/worker/app.py Outdated
@markstory markstory merged commit b1d60f6 into main Mar 31, 2026
22 checks passed
@markstory markstory deleted the fix-taskworker-secret branch March 31, 2026 16:05
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