Source API keys from environment/CI secrets instead of committed credentials - #627
Source API keys from environment/CI secrets instead of committed credentials#627Bill-Becker wants to merge 4 commits into
Conversation
…entials - Untrack test/.env and add it to .gitignore; add test/.env.example template - Supply NLR_DEVELOPER_API_KEY, URDB_API_KEY, and NLR_DEVELOPER_EMAIL to the CI test step from GitHub repository secrets - Remove hardcoded URDB API key fallback in src/core/urdb.jl in favor of get_urdb_api_key() which errors with guidance when URDB_API_KEY is unset - Document URDB_API_KEY setup and add CHANGELOG entry Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Updated changelog guidelines and sections for feature branches and merging practices. Added new pre-release section after v0.61.1.
There was a problem hiding this comment.
🟡 Changes recommended
The new URDB missing-key path uses throw(@error(...)), which won’t raise the intended exception, and the CI secret injection likely hard-fails for fork PRs where secrets are unavailable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR removes committed API credentials from the repository and switches both local testing and CI to source required API keys/emails from environment variables (and GitHub Actions secrets), with updated docs and changelog notes to reflect the new setup.
Changes:
- Stop tracking
test/.envand add an example templatetest/.env.examplefor local developer setup. - Remove the hardcoded URDB API key fallback and read
URDB_API_KEYfromENVat request time with a descriptive error when unset. - Update CI and docs to supply/describe required environment variables; add changelog entries for the change.
File summaries
| File | Description |
|---|---|
test/.env.example |
Adds a safe template for local test env vars without committing secrets. |
test/.env |
Removes previously committed credentials from version control. |
src/core/urdb.jl |
Switches URDB authentication to ENV["URDB_API_KEY"] and removes hardcoded fallback. |
docs/src/index.md |
Documents how to set URDB_API_KEY and how local tests load env vars from test/.env. |
CHANGELOG.md |
Records the credential-handling change and updates changelog workflow guidance. |
.gitignore |
Ensures .env files are ignored to prevent accidental secret commits. |
.github/workflows/CI.yml |
Injects NLR_DEVELOPER_API_KEY, URDB_API_KEY, and NLR_DEVELOPER_EMAIL via GitHub Actions secrets. |
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The CI workflow currently skips the test step for forked PRs while still reporting success, which can allow untested changes to merge unnoticed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
docs/src/index.md:25
urdb_rate_nameis referenced without theElectricTariff.prefix here, while the surrounding parameters use fully-qualified field names. Using the same prefix for both fields makes the docs easier to follow.
src/core/urdb.jl:11- The guidance in this error message uses Python-style quoting (single quotes) for
ENV[...], which is not valid Julia syntax and may confuse users following the instructions. Update the example toENV["URDB_API_KEY"] = "...".
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
Repository secrets are not exposed to pull_request runs from forks, so the previous `if:` guard skipped the test step while still reporting the job as successful. Replace it with an explicit pre-flight step that fails fast with an actionable message for fork PRs, so CI can never report green without having run the test suite. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Removes committed API credentials from the repo and sources them from environment variables / GitHub Actions secrets instead. Follows up on the
test/.envfinding from the Copilot review on #626.Changes
test/.env(git rm --cached) and add.envto.gitignore. Local developer files are unaffected.test/.env.exampletemplate with placeholders and signup URLs..github/workflows/CI.yml: add anenv:block on the test step mappingNLR_DEVELOPER_API_KEY,URDB_API_KEY, andNLR_DEVELOPER_EMAILfrom repo secrets.src/core/urdb.jl: remove the hardcoded URDB API key fallback. Replaced withget_urdb_api_key(), which readsENVand throws a descriptive error when unset, mirroringcheck_api_key()insrc/core/utils.jl. It is called at request time, so the package still loads without a key set.docs/src/index.mdplus a note about copyingtest/.env.exampletotest/.env.## Developsection.No real key values are committed anywhere.
Required before merge
NLR_DEVELOPER_API_KEY,URDB_API_KEY,NLR_DEVELOPER_EMAIL. CI will fail without them.