Module
registry/bpmct/modules/onepassword (v1.0.2)
Summary
The personal-account sign-in path automates op account add by feeding the user's 1Password account password through an expect script, sourced from a coder_parameter. While reviewing the module before adopting it, we found this path has security properties that don't look deliberate — and the original PR (#824) description only describes the manual flow ("User runs op signin in their terminal"), so the automated password path appears to have never been surfaced in review.
Question for @bpmct: was this trade-off intentional? If yes, a README warning would help; if not, we'd propose removing the path.
Details
-
The account password is persisted in the Coder database. data "coder_parameter" "account_password" (main.tf) means the password is stored server-side as a build parameter and pre-filled in workspace settings. Coder parameters are not a secrets mechanism.
-
The password is exposed in the process table and rendered script. run.sh interpolates ACCOUNT_PASSWORD into the text of the expect -c "..." program, which is passed as a command-line argument — visible in ps output to any process on the workspace while it runs. It is also baked into the rendered startup script via templatefile().
-
The session persistence adds risk without lasting benefit. The raw session token written to ~/.op/session and sourced from .bashrc/.zshrc expires after ~30 minutes, so the plumbing leaves a stale token on disk and permanent rc-file edits for a session that's gone within the hour.
(The service-account path is unaffected — that's the path we're using, and it works well.)
Proposed fix
Delete the automated password branch and let the module's own existing fallback be the only personal-account path — it already does the right thing:
printf "To sign in, run in your terminal:\n"
printf " op account add %s\n" "${ADD_ARGS}"
i.e. remove the account_password parameter from main.tf and the expect/session-file block from run.sh; keep address/email/secret-key pre-fill. op then prompts for the password interactively on first use (no echo, no storage, handled by the vendor binary).
Optional hardening while in there: on Debian/Ubuntu, install op from 1Password's official GPG-signed apt repository instead of the unverified zip download, falling back to the zip elsewhere.
Happy to open a PR with the above if the direction sounds right.
Module
registry/bpmct/modules/onepassword(v1.0.2)Summary
The personal-account sign-in path automates
op account addby feeding the user's 1Password account password through anexpectscript, sourced from acoder_parameter. While reviewing the module before adopting it, we found this path has security properties that don't look deliberate — and the original PR (#824) description only describes the manual flow ("User runsop signinin their terminal"), so the automated password path appears to have never been surfaced in review.Question for @bpmct: was this trade-off intentional? If yes, a README warning would help; if not, we'd propose removing the path.
Details
The account password is persisted in the Coder database.
data "coder_parameter" "account_password"(main.tf) means the password is stored server-side as a build parameter and pre-filled in workspace settings. Coder parameters are not a secrets mechanism.The password is exposed in the process table and rendered script. run.sh interpolates
ACCOUNT_PASSWORDinto the text of theexpect -c "..."program, which is passed as a command-line argument — visible inpsoutput to any process on the workspace while it runs. It is also baked into the rendered startup script viatemplatefile().The session persistence adds risk without lasting benefit. The raw session token written to
~/.op/sessionand sourced from.bashrc/.zshrcexpires after ~30 minutes, so the plumbing leaves a stale token on disk and permanent rc-file edits for a session that's gone within the hour.(The service-account path is unaffected — that's the path we're using, and it works well.)
Proposed fix
Delete the automated password branch and let the module's own existing fallback be the only personal-account path — it already does the right thing:
i.e. remove the
account_passwordparameter from main.tf and theexpect/session-file block from run.sh; keep address/email/secret-key pre-fill.opthen prompts for the password interactively on first use (no echo, no storage, handled by the vendor binary).Optional hardening while in there: on Debian/Ubuntu, install
opfrom 1Password's official GPG-signed apt repository instead of the unverified zip download, falling back to the zip elsewhere.Happy to open a PR with the above if the direction sounds right.