Skip to content

Conversation

@mattfield
Copy link
Contributor

@mattfield mattfield commented Jun 8, 2025

Adds support for providing a GitHub token via an environment variable:

  • Envvar passed in config: if there's a string value passed to token that begins with $, treat it as an envvar and perform a lookup
  • Known envvar present in environment: As per envvars in https://cli.github.com/manual/gh_help_environment, if there's no value set for token then look for GH_TOKEN, GITHUB_TOKEN, GH_ENTERPRISE_TOKEN, or GITHUB_ENTERPRISE_TOKEN, in that order, and use the first token found.
  • Any other string: treat it like a token.

I've also added a whole bunch of tests for as many scenarios/permutations of this that I can think of, as well as added serial_test as a dev dependency so the envvar tests run sequentially to avoid envvar definitions/removals clobbering each other. Please let me know if I've missed anything!

Closes #61

@danobi
Copy link
Owner

danobi commented Jun 10, 2025

Thanks for the PR! Acknowledging that I see it - I will review in the morning

- `token` field no longer accepts custom envvar names and we now only
  search for known GitHub envvars. If a located envvar is empty, we now
  return an error rather than continuing to search
- `resolve_github_token` refactored with a generic closure parameter for
  envvar lookups. Eliminates test race conditions with a custom lookup
  function rather than setting/wiping the environment each time,
  removing the need for tests to run sequentially
- Tidied up tests to remove old `resolve_github_token` implementation,
  and added new tests to cover token precedence, empty config fallback,
  etc
- Deduplicated docs in `config.md` in favour of linking, and clarified
  error message when GitHub token cannot be located
@mattfield
Copy link
Contributor Author

@danobi Many thanks for the review! Feedback has been implemented and PR updated. Specifics are in the commit message.

Copy link
Owner

@danobi danobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a bunch!

@danobi danobi merged commit e5076af into danobi:master Jun 12, 2025
3 checks passed
@mattfield mattfield deleted the support-resolving-token-from-envvar branch June 12, 2025 17:46
@mattfield
Copy link
Contributor Author

@danobi Pleasure!

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.

[FEATURE REQUEST] Accept a env variable at the config for token

2 participants