Skip to content

fix(oauth): validate OAuth token/base URLs and make user agent overrideable - #101

Merged
jaxxstorm merged 1 commit into
jaxxstorm:mainfrom
rshade:sec-gap
Aug 21, 2026
Merged

fix(oauth): validate OAuth token/base URLs and make user agent overrideable#101
jaxxstorm merged 1 commit into
jaxxstorm:mainfrom
rshade:sec-gap

Conversation

@rshade

@rshade rshade commented Aug 21, 2026

Copy link
Copy Markdown
Contributor
  • A downstream code review of pkg/tscli/pkg/oauth as a library found two ways an embedding consumer's secrets or metadata could leak: TSCLI_OAUTH_TOKEN_URL was read with no validation and checked before an explicit base-url, so it could silently redirect an OAuth client secret to an attacker-controlled host over plain HTTP; and the User-Agent sent on every API request was derived from the calling process's local git repository state with no way for a library consumer to override it.

  • pkg/oauth.ValidateTokenURL now requires https://, or http:// restricted to a loopback host, and is applied to both the OAuth token endpoint and base-url (pkg/tscli/client.go's validateBaseURL now delegates to it). The OAuth token endpoint is derived from the validated base-url first; TSCLI_OAUTH_TOKEN_URL remains a validated, explicit override, not a silent one.

  • New() also disables HTTP redirect following (CheckRedirect returns http.ErrUseLastResponse) so a redirected request can't carry the OAuth bearer token or API key to an unconfigured host.

  • Library and CLI callers can now set a user-agent viper key (flag/env TSCLI_USER_AGENT/config) to replace the git-derived default, so embedding a repo other than tscli no longer leaks that repo's tag, commit hash, and dirty state to Tailscale's API.

  • cmd/tscli/create/tailnet, cmd/tscli/create/token, cmd/tscli/delete/tailnet, and cmd/tscli/list/tailnets now call the new tscli.ExchangeOAuthClientCredentials, which derives the token URL from the same base-url configuration New() uses, instead of calling oauth.ExchangeClientCredentials directly with an unvalidated default.

  • pkg/contract/openapi and coverage/coverage-gaps.* were refreshed against Tailscale's live OpenAPI schema (make coverage-gaps-latest), picking up 3 new operations (tailnets list/create/delete) and a new vipServices[].displayName property; unrelated to the security fix.

  • pkg/oauth/exchange_test.go — new tests cover the default public token endpoint, a validated loopback override, rejection of an unsafe TSCLI_OAUTH_TOKEN_URL override, ValidateTokenURL, and ResolveTokenURL

  • pkg/tscli/client_test.go — new coverage for base-url/token-URL validation and resolution

  • test/cli/* — lifecycle and example-output integration tests updated to derive the OAuth token endpoint from TSCLI_BASE_URL instead of a separately set TSCLI_OAUTH_TOKEN_URL

  • make lint passes with zero issues

  • make test passes

  • pkg/oauth/exchange.go - Add ValidateTokenURL/ResolveTokenURL, require https:// (or loopback http://) for both the default and TSCLI_OAUTH_TOKEN_URL-overridden token endpoint

  • pkg/tscli/client.go - Validate base-url via ValidateTokenURL, derive the OAuth token endpoint from it, add ExchangeOAuthClientCredentials, support a user-agent viper override, and disable redirect following on the HTTP client

  • internal/cli/root.go - Bind TSCLI_USER_AGENT to the user-agent viper key

  • cmd/tscli/create/tailnet/cli.go, cmd/tscli/create/token/cli.go, cmd/tscli/delete/tailnet/cli.go, cmd/tscli/list/tailnets/cli.go - Use tscli.ExchangeOAuthClientCredentials instead of calling oauth.ExchangeClientCredentials directly

  • cmd/tscli/delete/users/cli_test.go - Use https://fake as the stub base URL to match the tightened base-url validation

  • README.md - Document base-url/user-agent validation and the TSCLI_USER_AGENT override

  • pkg/oauth/exchange_test.go, pkg/tscli/client_test.go, test/cli/example_output_test.go, test/cli/tailnet_lifecycle_integration_test.go, test/cli/unauthenticated_commands_test.go - Test coverage for the above

  • pkg/contract/openapi/tailscale-v2-openapi.yaml, pkg/contract/openapi/command-operation-map.yaml, pkg/contract/openapi/snapshot-metadata.yaml, coverage/coverage-gaps.json, coverage/coverage-gaps.md - Refresh the OpenAPI snapshot and coverage report against Tailscale's current API (make coverage-gaps-latest)

…dable

- A downstream code review of `pkg/tscli`/`pkg/oauth` as a library found
  two ways an embedding consumer's secrets or metadata could leak:
  `TSCLI_OAUTH_TOKEN_URL` was read with no validation and checked before
  an explicit `base-url`, so it could silently redirect an OAuth client
  secret to an attacker-controlled host over plain HTTP; and the
  `User-Agent` sent on every API request was derived from the calling
  process's local git repository state with no way for a library
  consumer to override it.
- `pkg/oauth.ValidateTokenURL` now requires `https://`, or `http://`
  restricted to a loopback host, and is applied to both the OAuth token
  endpoint and `base-url` (`pkg/tscli/client.go`'s `validateBaseURL` now
  delegates to it). The OAuth token endpoint is derived from the
  validated `base-url` first; `TSCLI_OAUTH_TOKEN_URL` remains a
  validated, explicit override, not a silent one.
- `New()` also disables HTTP redirect following (`CheckRedirect` returns
  `http.ErrUseLastResponse`) so a redirected request can't carry the
  OAuth bearer token or API key to an unconfigured host.
- Library and CLI callers can now set a `user-agent` viper key (flag/env
  `TSCLI_USER_AGENT`/config) to replace the git-derived default, so
  embedding a repo other than `tscli` no longer leaks that repo's tag,
  commit hash, and dirty state to Tailscale's API.
- `cmd/tscli/create/tailnet`, `cmd/tscli/create/token`,
  `cmd/tscli/delete/tailnet`, and `cmd/tscli/list/tailnets` now call the
  new `tscli.ExchangeOAuthClientCredentials`, which derives the token URL
  from the same `base-url` configuration `New()` uses, instead of calling
  `oauth.ExchangeClientCredentials` directly with an unvalidated default.
- `pkg/contract/openapi` and `coverage/coverage-gaps.*` were refreshed
  against Tailscale's live OpenAPI schema (`make coverage-gaps-latest`),
  picking up 3 new operations (tailnets list/create/delete) and a new
  `vipServices[].displayName` property; unrelated to the security fix.

- [x] `pkg/oauth/exchange_test.go` — new tests cover the default public
      token endpoint, a validated loopback override, rejection of an
      unsafe `TSCLI_OAUTH_TOKEN_URL` override, `ValidateTokenURL`, and
      `ResolveTokenURL`
- [x] `pkg/tscli/client_test.go` — new coverage for `base-url`/token-URL
      validation and resolution
- [x] `test/cli/*` — lifecycle and example-output integration tests
      updated to derive the OAuth token endpoint from `TSCLI_BASE_URL`
      instead of a separately set `TSCLI_OAUTH_TOKEN_URL`
- [x] `make lint` passes with zero issues
- [x] `make test` passes

- `pkg/oauth/exchange.go` - Add `ValidateTokenURL`/`ResolveTokenURL`,
  require `https://` (or loopback `http://`) for both the default and
  `TSCLI_OAUTH_TOKEN_URL`-overridden token endpoint
- `pkg/tscli/client.go` - Validate `base-url` via `ValidateTokenURL`,
  derive the OAuth token endpoint from it, add
  `ExchangeOAuthClientCredentials`, support a `user-agent` viper
  override, and disable redirect following on the HTTP client
- `internal/cli/root.go` - Bind `TSCLI_USER_AGENT` to the `user-agent`
  viper key
- `cmd/tscli/create/tailnet/cli.go`, `cmd/tscli/create/token/cli.go`,
  `cmd/tscli/delete/tailnet/cli.go`, `cmd/tscli/list/tailnets/cli.go` -
  Use `tscli.ExchangeOAuthClientCredentials` instead of calling
  `oauth.ExchangeClientCredentials` directly
- `cmd/tscli/delete/users/cli_test.go` - Use `https://fake` as the stub
  base URL to match the tightened `base-url` validation
- `README.md` - Document `base-url`/`user-agent` validation and the
  `TSCLI_USER_AGENT` override
- `pkg/oauth/exchange_test.go`, `pkg/tscli/client_test.go`,
  `test/cli/example_output_test.go`,
  `test/cli/tailnet_lifecycle_integration_test.go`,
  `test/cli/unauthenticated_commands_test.go` - Test coverage for the
  above

- `pkg/contract/openapi/tailscale-v2-openapi.yaml`,
  `pkg/contract/openapi/command-operation-map.yaml`,
  `pkg/contract/openapi/snapshot-metadata.yaml`,
  `coverage/coverage-gaps.json`, `coverage/coverage-gaps.md` - Refresh
  the OpenAPI snapshot and coverage report against Tailscale's current
  API (`make coverage-gaps-latest`)
@jaxxstorm
jaxxstorm merged commit 3e58d42 into jaxxstorm:main Aug 21, 2026
1 check passed
@rshade
rshade deleted the sec-gap branch August 21, 2026 18:08
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.

2 participants