coder / coder/registry

bpmct/onepassword: automated personal-account sign-in stores the account password in Coder's parameter DB and exposes it in the process table

Open
#1,023 1 comment 0 reactions 1 assignee Claimed by @bpmct View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.