Skip to content

workshopctl: add systemd secret retrieval - #996

Open
tlm wants to merge 5 commits into
secrets-implementationfrom
workshopctl-get-secret
Open

workshopctl: add systemd secret retrieval#996
tlm wants to merge 5 commits into
secrets-implementationfrom
workshopctl-get-secret

Conversation

@tlm

@tlm tlm commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Add the workshopctl get-secret plumbing used by systemd
LoadCredential= requests.

The implementation has two paths:

  • Plain workshopctl get-secret <sdk>.<secret> forwards a secret request to
    the daemon.
  • workshopctl get-secret --systemd handles a request from the Workshop
    secret socket. It reads the peer address of the accepted Unix connection to
    identify the requesting systemd unit and credential, then forwards a normal
    <sdk>.<secret> request to the daemon.

The systemd path writes only credential bytes to stdout. Diagnostics go to
stderr, allowing the socket-activated unit to route them to the journal
without contaminating the credential value.

Systemd behaviour

A systemd peer address has the form:

\0<random>/unit/<unit>/<sdk>.<secret>

For example:

\0<random>/unit/ollama.service/ollama.ollama-api-key

workshopctl decodes this into the requesting unit, SDK, and secret plug.
The systemd response handler implements the secret-spec exit codes:

Result Exit code Credential output
Plug not connected 0 Zero-byte credential
Secret not found 1 None
Secret provider locked 2 None
Other failure 255 None
Success 0 Secret value

Current scope

The daemon command currently returns the placeholder value:

workshop-placeholder-secret

It logs the requested secret identifier but never its value. Host secret
provider lookup and secret-interface connection resolution are follow-up work.

This PR depends on the base branch's Workshop secret socket units.

Client refactor

WorkshopCtlOptions now owns its optional stdin reader. This lets local
interceptors consume or clear stdin before forwarding the request to the
daemon. The systemd interceptor clears it after reading the peer address, so
the accepted socket is not buffered into the API request body.

Testing

Added coverage for:

  • plain get-secret invocation and non-root access;
  • required secret argument validation;
  • systemd peer-address parsing and malformed input;
  • decoding a peer address from a real Unix socket;
  • systemd success and error response handling;
  • interception of get-secret --systemd;
  • client stdin size limit handling.

Validated with:

go test ./client ./cmd/workshopctl ./internal/overlord/hookstate/ctlcmd

Manual QA

Requires a Workshop build containing both this PR and the Workshop secret
socket units.

  1. Launch a new workshop, then enter it as root.

  2. Confirm the resolver socket is active:

    systemctl is-active workshop-secret.socket

    Expected output:

    active
    
  3. Create a one-shot system service that requests a credential and writes it
    to a temporary file:

    cat >/etc/systemd/system/workshop-secret-smoke.service <<'EOF'
    [Unit]
    Description=Workshop secret smoke test
    After=workshop-secret.socket
    Wants=workshop-secret.socket
    
    [Service]
    Type=oneshot
    LoadCredential=smoke-test.api-token:/var/lib/workshop/run/workshop.socket.secret
    ExecStart=/bin/sh -c 'cat "$CREDENTIALS_DIRECTORY/smoke-test.api-token" > /tmp/workshop-secret-smoke-value'
    EOF
    
    systemctl daemon-reload
    systemctl start workshop-secret-smoke.service
  4. Confirm systemd received the credential:

    cat /tmp/workshop-secret-smoke-value

    Expected output for the current placeholder implementation:

    workshop-placeholder-secret
    
  5. Confirm the socket-activated resolver processed the request:

    systemctl list-units --all 'workshop-secret@*.service'

    Copy an instance name from the output, then inspect its log:

    journalctl --no-pager -u <workshop-secret-instance>

    The log should identify the requesting unit, SDK, and secret name, but must
    not include the credential value.

  6. Confirm the plain command is authorised as the Workshop user:

    sudo -u workshop workshopctl get-secret smoke-test.api-token

    Expected output:

    workshop-placeholder-secret
    

This verifies the success path and systemd transport only. Host keyring lookup,
plug connection state, missing secrets, and locked providers require the future
daemon-side secret resolver.

Docs

  • I confirm the PR has no implications for documentation.

tlm added 4 commits August 20, 2026 00:41
stdinReadLimit is a fixed protection against buffering too much stdin
data into the request body, so express it as a const with an
underscore-separated literal rather than a var. Document why the
limit exists: stdin is currently buffered in full rather than
streamed, so the request body size must be bounded.

The var only existed so tests could mock the limit down to 10 bytes.
Drop MockStdinReadLimit and have the read-limit tests exercise the
real 4MB boundary instead; the fixtures allocate a single slice and
run in milliseconds, so the mock added indirection without benefit.
ConstError is a string-based error type for declaring sentinel errors
as constants. Unlike sentinels created with errors.New, a ConstError
cannot be reassigned and is matched by errors.Is through string
equality, so each message must be unique.

Convert ErrorNoWaitingChange to the new type as its first user.
Introduce local interception in workshopctl: invocations are inspected
before being forwarded to the daemon, allowing subcommands to alter
the request and install a response handler that produces the process
exit code. The default handler preserves the existing pass-through
behaviour.

Use this for get-secret --systemd, invoked by the workshop-secret
socket unit with the accepted connection on stdin. The LoadCredential
peer address is decoded locally to identify the requesting unit, sdk
and secret, then forwarded to the daemon as a regular get-secret
invocation with stdin cleared, since the connection was consumed
locally. The response handler maps daemon errors to the exit codes
defined by the secrets spec: plug not connected yields a zero-byte
credential (exit 0), secret not found exits 1, locked provider exits
2, and any other error exits 255.

Support this in the client by splitting the workshopctl options from
the wire type: WorkshopCtlPostData now defines the JSON transport with
flat members, and WorkshopCtlOptions carries Stdin as an io.Reader so
RunWorkshopctl takes a single options argument. Add ConstError
sentinels for the plug-not-connected, secret-not-found and
provider-locked outcomes so the handler can match them with errors.Is.
Add the daemon side of workshopctl get-secret: the subcommand resolves
a secret identified as "<sdk>.<secret>" and writes the value to
stdout. Resolution via workshopd is not implemented yet, so a
hard-coded placeholder value is returned; the requested identifier is
recorded with a debug log (never the value).

Allow get-secret to run without root, as both the socket-activated
--systemd service and SDK wrapper scripts invoke workshopctl as the
workshop user.
@tlm
tlm requested a review from dmitry-lyfar August 20, 2026 01:53
@tlm tlm self-assigned this Aug 20, 2026
These are convenience wrappers where callers only care about success or
failure. Returning the byte count from fmt.Fprintf serves no purpose at
this level and was causing the unparam linter to flag printf once it
gained its first non-test caller.
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.

1 participant