Skip to content
This repository was archived by the owner on Oct 13, 2026. It is now read-only.

feat: nook secret set, list, and unset - #6

Merged
vish727 merged 3 commits into
mainfrom
eng-24-secret-commands
Sep 22, 2026
Merged

vish727 merged 3 commits into
mainfrom
eng-24-secret-commands

Conversation

@vish727

@vish727 vish727 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

The CLI half of step 2 of DESIGN.md §10b. Pairs with the nookd PR that stores them.

$ nook secret set board SLACK_TOKEN
value for SLACK_TOKEN (not shown):
SLACK_TOKEN is set for board. its value is never shown again.
  SLACK_TOKEN  set by vishal@acme.com

$ printf '%s' "$TOKEN" | nook secret set board SLACK_TOKEN   # or from a pipe
  • The value is never an argument. argv lands in shell history and is visible in ps, so it comes from a pipe or a prompt and nothing else.
  • Echo is turned off through stty, which keeps nook-cli's dependency list empty rather than pulling in x/term for one prompt. Where echo cannot be turned off the CLI says so rather than quietly showing the value.
  • list shows declared against set, so the gap that stops a nook from starting is the thing you see, with who set each one. No command prints a value, including --json.
  • An uppercase KEY is not mistaken for the nook's name, so nook secret set SLACK_TOKEN works inside a nook folder.

No MCP tool for this, deliberately. The other eleven have one. A secret's value passing through an agent's tool call is the thing this feature exists to avoid, and an agent that needs it can tell the person the one command to run.

Part of ENG-24

A static nook's files reach every viewer's browser, so they are readable by
anyone who can open it. A service nook's run code never reaches a browser, and
should not become readable just because someone was shared the nook.

source is "editors" (owner and editors) by default, or "viewers" to opt in to
the static behaviour. Parse writes the default out explicitly, so a pull or an
export of a service nook states who can read it rather than leaving it implied.

nookd enforces this on /_nook/source, pull, and remix.
A value is never an argument: argv lands in shell history and is visible in ps.
It comes from a pipe, or from a prompt with the terminal's echo off through
stty, which keeps the CLI's dependency list empty for the sake of one prompt.
Where echo cannot be turned off the CLI says so instead of quietly showing it.

list shows what nook.json declares against what is set and by whom, so the gap
that stops a nook starting is the thing you see. No command prints a value.
@vish727

vish727 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Reviewed. The shape is right: the value never reaches argv, list shows declared against set, and no path prints a value. Three things before this merges.

A piped value is truncated at the first newline. readSecretValue reads one line, so cat service-account.json | nook secret set board GOOGLE_KEY stores only the first line and says it succeeded. A PEM key and a JSON credential are both normal things to pipe in, and losing the rest silently is the worst version of this bug: the nook fails at runtime with a value that looks set. Read all of stdin and strip at most one trailing newline.

Echo stays off if the prompt is interrupted. echoOff restores through the returned closure, so Ctrl+C at the prompt leaves the terminal with -echo and the person types blind until they run stty echo. Catch SIGINT for the duration of the prompt, restore, then exit.

isSecretName and the server's rule disagree at the edges. The CLI accepts _FOO and 123 as a key, the server's regexp requires a leading uppercase letter, so those become a round trip to a 400. Worth matching the regexp so the error arrives before the request.

Nothing else. The reasoning on no MCP tool for this is right, and worth keeping in the docs when they land.

…s here

A piped value was read one line at a time, so a PEM key or a JSON credential
stored its first line and reported success: the nook then fails at runtime with
a value that looks set. Stdin is now read whole, with exactly one trailing
newline removed, the one a shell or an editor adds.

Ctrl+C at the prompt left the terminal with -echo, typing blind until stty echo.
SIGINT is caught for the length of the prompt, restores, and exits 130.

A name the server's regexp rejects is refused here instead of as a round trip to
a 400. isSecretName stays looser on purpose, so "_FOO" is read as a misspelled
key and named as one rather than taken for a nook.

go test ./cmd/nook covers the pipe, since a stored value can never be read back.
@vish727

vish727 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

All three fixed.

Piped values are read whole, with exactly one trailing newline removed — the one a shell or editor adds — and everything else kept. You were right that this was the worst version of the bug: it reported success.

Since a stored value can never be read back, there is nowhere downstream to observe this, so it is covered by a unit test instead: a 4-line PEM key round-trips intact, along with abc\n\n → abc\n, CRLF, no trailing newline, inner whitespace, and pretty-printed JSON. Empty input and a lone newline are still refused.

SIGINT is caught for the length of the prompt, restores echo, and exits 130.

Names are checked against the server's regexp before the request. One deliberate difference from your suggestion: I left isSecretName — the thing that tells a KEY from a nook name during argument parsing — looser than the rule. If it matched the regexp exactly, nook secret set _FOO would read _FOO as the nook's name and fail with "usage:", which is a worse error than the one it has now. So _FOO and 123 are still recognised as intended keys and get named as bad keys. Both paths are tested.

Noted on the MCP reasoning going in the docs — that lands with the SKILL.md changes in step 5, where the helper libraries are documented.

@vish727
vish727 merged commit 19d4fda into main Sep 22, 2026
1 check passed
@vish727
vish727 deleted the eng-24-secret-commands branch September 22, 2026 01:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant