feat(relay): add resource auth enrollment for relays#237
feat(relay): add resource auth enrollment for relays#237saifsmailbox98 wants to merge 2 commits into
Conversation
Add --enroll-method flag to relay start command supporting token and AWS auth methods, mirroring the gateway enrollment pattern. - Token auth: --enroll-method=token --token=<enrollment-token> exchanges one-time token for RELAY_ACCESS_TOKEN, stored on disk - AWS auth: --enroll-method=aws --relay-id=<id> signs STS GetCallerIdentity, gets fresh token on each start - Stored config at /etc/infisical/relays/<name>.conf (root) or ~/.infisical/relays/<name>.conf (non-root) - v2 connect endpoint for PKI certs, v2 heartbeat endpoint - --host no longer required when using --enroll-method (host is stored server-side at relay creation time) - Legacy machine identity flow (--token without --enroll-method) unchanged for backward compatibility
|
💬 Discussion in Slack: #pr-review-cli-237-feat-relay-add-resource-auth-enrollment-for-relays Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
…or token path - Reject unknown --enroll-method values early with a clear error - Add LoadStoredDomain fallback in token enrollment path (mirrors AWS path), fixing re-enrollment without --domain on self-hosted
|
@claude review once |
There was a problem hiding this comment.
Both previously flagged issues look correctly addressed in the latest revision — token-path now uses the same LoadStoredDomain fallback as the AWS path, and invalid --enroll-method values are rejected up front. Deferring approval to a human since this is non-trivial auth/enrollment code.
Extended reasoning...
Overview
The PR adds a new --enroll-method flag to infisical relay start supporting two enrollment modes:
token: one-time enrollment token exchanged viaPOST /v2/relays/loginfor a persisted access tokenaws: AWS STSGetCallerIdentitysigned-request login viaPOST /v2/relays/login
Touches: new resty wrappers (CallRelayLogin, CallRelayConnect, CallRelayHeartbeatV2) in packages/api/api.go, request/response models in packages/api/model.go, the relay start command in packages/cmd/relay.go, and two new files: packages/relay/aws_auth.go (SigV4 signing for STS) and packages/relay/enroll.go (on-disk relay config: access token, enrollment token, domain, relay id). packages/relay/relay.go is updated to dispatch heartbeat/register through the v2 endpoints when an enroll method is set.
Security risks
- New persisted-secret storage in
packages/relay/enroll.gowrites access tokens and enrollment tokens to/etc/infisical/relays/<name>.conf(root) or~/.infisical/relays/<name>.conf(non-root) with0600perms and0700directory perms. Looks reasonable but worth a human eye on the format (plainKEY=VALUE, no escaping — values containing newlines would corrupt the file, though tokens normally won't). - The token flag is now overloaded between legacy machine-identity tokens and one-time enrollment tokens, distinguished only by
--enroll-method. Help text covers it but the dual meaning increases footgun surface. - AWS SigV4 path constructs and base64-encodes signed STS request headers and forwards them to the Infisical backend — standard AWS-auth pattern used elsewhere in the codebase (mirrors gateway AWS auth).
- Global mutation of
config.INFISICAL_URLbased on stored state means re-runs effectively trust the on-disk<name>.conffor the backend URL; a writable-as-user attacker who can modify the conf could redirect enrollment to a malicious backend. Probably acceptable given the conf is in the user's home dir, but worth confirming.
Level of scrutiny
Medium-high. This is new authentication code on a critical path (relay enrollment talks to the production backend with bearer tokens), introduces new on-disk secret storage, and overloads an existing flag. Not a config tweak or refactor. The companion backend PR (infisical#6515) defines the server side, so the contract should be reviewed jointly — that's not something an automated reviewer can do well.
Other factors
- Both previously flagged bugs were addressed with minimal, targeted diffs that match the suggested fix.
- There is some redundancy: the domain resolution block at the end of the dispatch also runs after both per-method blocks did the same thing, but it is benign (idempotent through
AppendAPIEndpoint). - I'd want a human to verify the response shape of
CallRelayConnectagainst the backend PR and confirm the v2 heartbeat unauthenticated-vs-authenticated semantics match the access-token-based auth model.
Description 📣
Add --enroll-method flag to relay start command for token and AWS auth. Companion to Infisical/infisical#6515.
Backend PR: Infisical/infisical#6515
Type ✨