feat(cloudflare): add Cloudflare API token lease backend#335
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the credential leasing capabilities by integrating a new Cloudflare API token lease backend. This feature allows users to securely provision temporary, finely-scoped API tokens for Cloudflare services, improving security posture by minimizing the lifetime and permissions of access credentials. It streamlines the process of managing Cloudflare access within automated workflows. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new lease backend for Cloudflare API tokens, allowing for the creation of short-lived, scoped tokens. The implementation includes token creation, revocation, and configuration options for policies and account IDs. The changes are well-documented with a dedicated page for the new backend and an example in the main lease guide. The code is well-structured and includes good error handling. I have one suggestion to simplify the JSON serialization logic by leveraging serde's derived implementations, which will improve code maintainability.
| "permission_groups": p.permission_groups.iter().map(|pg| { | ||
| let mut m = serde_json::Map::new(); | ||
| m.insert("id".to_string(), serde_json::Value::String(pg.id.clone())); | ||
| if let Some(name) = &pg.name { | ||
| m.insert("name".to_string(), serde_json::Value::String(name.clone())); | ||
| } | ||
| serde_json::Value::Object(m) | ||
| }).collect::<Vec<_>>(), |
There was a problem hiding this comment.
The manual JSON construction for permission_groups is unnecessary because the CloudflarePermissionGroup struct derives serde::Serialize. You can simplify this by letting serde_json handle the serialization directly within the json! macro. This improves readability and maintainability.
"permission_groups": p.permission_groups,d65c122 to
fdc4a5a
Compare
Greptile SummaryThis PR adds a The implementation is well-structured and addresses most of the common pitfalls in this class of API-backed credential backend (enum-based effect validation, character-aware token name truncation, saturating cast for duration, explicit guards for empty
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant fnox as fnox CLI
participant Ledger as LeaseLedger
participant CF as Cloudflare API
User->>fnox: fnox lease create cf
fnox->>fnox: resolve_secrets_batch()
fnox->>fnox: check_prerequisites() - verify parent token present
alt policies configured in fnox.toml
fnox->>fnox: build_api_policies(policies, account_id)
else no policies - inherit from parent
fnox->>CF: GET tokens/verify
CF-->>fnox: parent token_id
fnox->>CF: GET tokens/{token_id}
CF-->>fnox: policies (API Tokens perms stripped)
end
fnox->>CF: POST tokens<br/>with name, policies, expires_on
CF-->>fnox: child token id and value
fnox->>Ledger: record lease (lease_id = CF token UUID)
fnox-->>User: child token credential
User->>fnox: fnox lease revoke {lease_id}
fnox->>Ledger: find record by CF UUID
fnox->>CF: DELETE tokens/{lease_id}
CF-->>fnox: 200 OK or 404 treated as success
fnox->>Ledger: mark_revoked(lease_id)
fnox-->>User: Lease revoked
|
|
bugbot run |
|
bugbot run |
1ebe16f to
8f4ef10
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Creates short-lived, scoped Cloudflare API tokens using the Cloudflare API Tokens API. A parent token with 'API Tokens: Edit' permission creates child tokens that are scoped to specified permission policies and automatically expire. Supports explicit revocation via token deletion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use CloudflarePolicyEffect enum instead of String for effect field - Change resources map values from serde_json::Value to String - Use serde derive for permission_groups serialization - Fix empty errors array producing blank error messages - Truncate token name to Cloudflare's 100-char limit - Validate policies is non-empty before API call - Remove not_before field (Cloudflare defaults to immediate validity) - Saturate u64→i64 cast for duration - Mention CF_API_TOKEN in required_env_vars hint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When `policies` is omitted from the config, the Cloudflare backend now
fetches the parent token's policies via the API (GET /user/tokens/verify
then GET /user/tokens/{id}) and uses them for the child token. This
makes the minimal config just `type = "cloudflare"`.
Changed `policies` from `Vec` (defaulting to empty) to `Option<Vec>` so
the intent is explicit: `None` = inherit, `Some([...])` = override.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…id} placeholders
When `account_id` is set, the backend now uses the account-owned tokens
API (`/accounts/{id}/tokens`) instead of `/user/tokens`. Account tokens
are better for CI/CD since they aren't tied to individual users.
Also adds a guard that errors early if a resource key contains the
`{account_id}` placeholder but `account_id` is not configured, instead
of sending the literal placeholder string to the API.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds an explicit `token_type` config field ("user" or "account") instead
of implicitly choosing based on `account_id` presence. When set to
"account", `account_id` is required and the backend uses the
account-owned tokens API.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cloudflare forbids sub-tokens from having token management permissions. When inheriting policies from the parent token, filter out any "API Tokens" permission groups to avoid the "sub-token is not allowed to have permissions to manage other tokens" error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Always use /user/tokens for verify/details (account endpoint doesn't exist) - Sanitize inherited policies: only extract effect, resources, and permission group id (drop server-generated fields like status, created_on) - Fix UTF-8 byte-boundary panic on token name truncation (use char-based) - Validate empty policies and per-policy empty permission_groups - Add CF_API_TOKEN to required_env_vars for secret resolution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests share a global SOURCES static and run in parallel. Calling clear() in each test creates a race where one test's clear() can wipe another test's registered data. Since each test uses unique paths, clear() is unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The verify and token details endpoints didn't check HTTP status codes before extracting JSON fields. Invalid/expired tokens (401/403) produced misleading "missing result.id" errors instead of proper auth errors. Extracted cf_api_call helper that checks status and returns proper ProviderAuthFailed for 401/403 responses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a secret required by a lease backend fails to resolve (e.g. FIDO2 PIN error), resolve_secrets_batch respects if_missing (default: "warn") and swallows the error. This produced a misleading "token not found" from check_prerequisites instead of surfacing the actual auth failure. Now checks if any required secrets resolved to None and fails early with a clear error message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Account-scoped tokens must verify via /accounts/{id}/tokens/verify,
not /user/tokens/verify. The greptile review incorrectly claimed the
account verify endpoint doesn't exist — it does. Reverts to passing
tokens_path to fetch_parent_policies.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CF_API_TOKEN is a runtime fallback alias, not an independently required var. Listing both caused the hard-fail check to fire if only one was defined as a managed secret and it failed to resolve. Also fix CI build: move the hard-fail check into create_single where backend_config exists, passing resolved_secrets through run_all. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fdc5d4a to
8b91bc7
Compare
…ckends Each backend module now owns its prerequisite checks and env var definitions. mod.rs dispatches to them instead of embedding env var names directly. This centralizes backend-specific knowledge in the backend modules where it belongs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…single Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
### 🚀 Features - **(cloudflare)** add Cloudflare API token lease backend by [@jdx](https://github.com/jdx) in [#335](#335) - **(fido2)** bump demand to v2, mask PIN during typing by [@jdx](https://github.com/jdx) in [#334](#334) - **(init)** add -f as short alias for --force by [@jdx](https://github.com/jdx) in [#329](#329) - **(lease)** add --all flag, default to creating all leases by [@jdx](https://github.com/jdx) in [#337](#337) - **(lease)** add GitHub App installation token lease backend by [@jdx](https://github.com/jdx) in [#342](#342) ### 🐛 Bug Fixes - **(config)** fix directory locations to follow XDG spec by [@jdx](https://github.com/jdx) in [#336](#336) - **(exec)** use unix exec and exit silently on subprocess failure by [@jdx](https://github.com/jdx) in [#339](#339) - **(fido2)** remove duplicate touch prompt by [@jdx](https://github.com/jdx) in [#332](#332) - **(set)** write to lowest-priority existing config file by [@jdx](https://github.com/jdx) in [#331](#331) - **(tui)** skip providers requiring interactive auth by [@jdx](https://github.com/jdx) in [#333](#333) ### 🛡️ Security - **(ci)** retry lint step to handle transient pkl fetch failures by [@jdx](https://github.com/jdx) in [#341](#341) - **(mcp)** add MCP server for secret-gated AI agent access by [@jdx](https://github.com/jdx) in [#343](#343) - add guide for fnox sync by [@jdx](https://github.com/jdx) in [#328](#328) ### 🔍 Other Changes - share Rust cache across CI jobs by [@jdx](https://github.com/jdx) in [#340](#340)
Summary
cloudflarelease backend that creates short-lived, scoped Cloudflare API tokens via the Cloudflare API Tokens API{account_id}placeholder substitution in resource keys, and configurable env var namedocs/leases/cloudflare.md) and updates the lease guide with a Cloudflare exampleTest plan
cargo checkpasses (confirmed locally)cloudflarelease, runfnox lease create, verify scoped token works and expiresfnox lease revokedeletes the token via the Cloudflare APICLOUDFLARE_API_TOKENCloudflarevariant🤖 Generated with Claude Code
Note
Medium Risk
Introduces a new external-API-backed credential issuer (Cloudflare token create/delete) and changes lease creation error handling, which could affect lease provisioning flows and failure modes.
Overview
Adds a new
cloudflarelease backend that vends short-lived, scoped Cloudflare API tokens (user- or account-owned), supports optional explicitpolicies(or inheritance from the parent token),{account_id}substitution, configurable env var output, and revocation via token deletion.Updates configuration/schema and docs to expose the new backend, and refactors prerequisite/interactive env-var guidance into per-backend helpers.
Strengthens
fnox lease createby hard-failing when secrets required by a backend fail to resolve (instead of continuing and later reporting a generic “token not found”), reducing confusing auth error paths.Written by Cursor Bugbot for commit 2dbc9e3. This will update automatically on new commits. Configure here.