bpmct/onepassword: automated personal-account sign-in stores the account password in Coder's parameter DB and exposes it in the process table
- Dominant language
- HCL
- Stars
- 79
- Forks
- 161
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 38
Description
### 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
1. **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.
2. **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()`.
3. **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:
```sh
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.
Contributor guide
Assessment
This issue has not been assessed yet.