Repository navigation
feat(credentials): saved credentials for 21 token-and-host blocks on one host-aware check factory - #9849
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
 [High impact] The reviewed changes appear safe to merge; no blocking issue was found. SummaryAdds saved credentials and credential-only successors for 21 blocks, using one host-aware token-check factory.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 504 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Turn on auto-fix | Re-trigger cubic
fb59722 to
c5b0bbc
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 506 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Turn on auto-fix | Re-trigger cubic
…r factory tokenCheckValidator now takes a domain normalizer (stored as the credential's domain), an account-id normalizer, an SSRF-guarded egress profile for hosts the user chose, and a flag that keeps token-echoing error bodies out of logs. GitLab becomes a declaration on it, so every token check reads the same way.
Algolia, Azure DevOps, Convex, Devin, Dynatrace, Email Bison, Grafana, Infisical, Jotform, Jupyter (v3), Logfire, Mailgun, New Relic, Okta, Plane, Supabase, Tinybird, Twilio SMS and Voice (one shared Twilio credential), and Upstash get a credential that carries the token plus its host, region, or account id, and a credential-only successor. Every host-deciding param comes from the credential; a typed host is normalized as the tools do it and verified through the guarded egress, a region or account id is checked before any request, and Databricks subject tokens last up to an hour so long Genie polls keep their token.
…it findings Thrive gets a credential holding its key, tenant id, and region host (one of its four API hosts). Audit fixes: the connect form sends URL-shaped domains as typed so the provider check keeps their scheme and path; credentials default to a name with their domain or account id (GitLab keeps its token name) and record it in the audit log; Upstash accepts http and a read-only token refused PING; Infisical keeps http and a proxy path; Devin, Dynatrace, and Logfire refuse dot segments, non-http schemes, and mistyped regions.
…ecks One normalizeHttpBaseUrl and one allowedDomain replace five URL normalizers and four region checks (fixing Grafana and Tinybird treating a non-http scheme as a host); region tables come from the tools; acceptsBody can name the credential (GitLab); Email Bison and Jupyter refuse credentials in the URL instead of dropping them; the Supabase public-URL policy is inlined; the Databricks token lifetime uses the issuer's cap.
Azure DevOps tools build every project-scoped URL through one traversal-safe helper, so a project or build id can't climb into another organization with a saved token; Tinybird and Upstash restricted-token replies are covered by tests.
An Azure DevOps project or organization pasted already encoded is decoded once before the traversal-safe segment, so it isn't encoded twice; a base URL with a single-slash scheme is refused; Upstash accepts a proxy path like the legacy block; a Thrive tenant id must be printable ASCII.
c5b0bbc to
e02e8d9
Compare
…tion skills add-integration, add-block, add-tools, and validate-integration now describe the token credential (descriptor, tokenCheckValidator declaration with its host and account normalizers, shared policy mapping every host-deciding param) and credential-only successors for blocks that take a secret inline.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 510 files
Confidence score: 4/5
- In
.agents/skills/add-tools/SKILL.md, the example uses a nonexistentSERVICE_CREDENTIAL_OAUTHfactory, so copying it can fail to compile. Point readers totokenCredentialOAuthinstead. - In
.agents/skills/validate-integration/SKILL.md, the checklist requirestokenCredentialOAutheven for ordinary browser-OAuth integrations. Limit that requirement to token-credential integrations. - In
.agents/skills/add-integration/SKILL.md, requiringAuthMode.OAuthfor token credentials conflicts with the token-onlyAuthMode.ApiKeypath. Make the auth-mode choice conditional. - In
.agents/skills/validate-integration/SKILL.md, the check applies to legacy blocks that intentionally keep inline password inputs. Scope it to credential-only successor blocks.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".agents/skills/validate-integration/SKILL.md">
<violation number="1" location=".agents/skills/validate-integration/SKILL.md:221">
P3: This checklist item applies to every block, but legacy blocks keep their inline `password` inputs by design. Scope it explicitly to credential-only successor blocks, e.g. 'For a credential-only successor, a pasted key comes from a saved token credential...', so auditors don't flag legacy blocks.</violation>
<violation number="2" location=".agents/skills/validate-integration/SKILL.md:224">
P2: This checklist unconditionally requires `tokenCredentialOAuth` for every tool, including ordinary browser-OAuth integrations. Qualify it as a requirement for token-credential integrations so the normal `oauth: { required: true, provider }` contract remains valid.</violation>
</file>
<file name=".agents/skills/add-tools/SKILL.md">
<violation number="1" location=".agents/skills/add-tools/SKILL.md:93">
P2: `SERVICE_CREDENTIAL_OAUTH` does not exist in `tools/shared/saved-credentials.ts`, so copying this snippet fails to compile. Point readers to the real factory, `tokenCredentialOAuth('{service}', { apiKey: 'accessToken' })`, and say the mapping keys are the tool's own token/host/account params.</violation>
</file>
<file name=".agents/skills/add-integration/SKILL.md">
<violation number="1" location=".agents/skills/add-integration/SKILL.md:94">
P2: This step unconditionally requires `AuthMode.OAuth` for token credentials, contradicting the later token-only `AuthMode.ApiKey` path and its required availability registration. Make the auth-mode choice conditional on the credential flow, so token-only services retain the documented API-key path.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Turn on auto-fix | Re-trigger cubic
The token-credential skill steps name the real tokenCredentialOAuth factory, scope the checklist to token-credential and successor blocks, and keep the AuthMode.ApiKey path for token-only blocks; normalizeEmailBisonBaseUrl gains its TSDoc.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 510 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Turn on auto-fix | Re-trigger cubic
…ps scheme normalizeEmailBisonBaseUrl now keeps any typed scheme, so ftp://host is refused instead of becoming a host named ftp.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 510 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
…l executor - A policy that maps credential fields owns them: the generic domain, instanceUrl, apiDomain and cloudId fills no longer write a stored host into a same-named user param (Thrive create/update user sent its region host as the user's domain). - An agent tool with a saved credential selected is neither asked for nor allowed to send the params the credential supplies (Azure DevOps organization, Infisical base URL). - ENOTFOUND is read from the error or its cause, so a mistyped host or account id is site_not_found under Bun as well as Node. - A service-account secret is released only to a tool bound to its provider; a bare credential ID is refused.
…s and copy - One refuseUnsafeTypedUrl guard refuses a typed instance URL with credentials or a non-http(s) scheme, for Email Bison, Jupyter and Logfire, before their normalizers can read the scheme as a host. The Email Bison tool normalizer is back to its staging behavior; the check lives in the credential validator, so legacy blocks are unchanged. - Docs for successor users: Jotform region, Dynatrace scopes, Logfire host, Plane instance, Infisical, Fathom and Supabase now point at the saved credential; self-hosting no longer says API keys live in blocks; the AWS keyless step names the dialog's Issuer. - ElevenLabs and Instantly copy no longer asks for a permission their checks don't need; Okta lists trexcloud.com.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 519 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
Summary
21 more blocks get a saved credential and a credential-only version. This time the connection is a token plus its host, region, or account ID: Algolia, Azure DevOps, Convex, Devin, Dynatrace, Email Bison, Grafana, Infisical, Jotform, Jupyter (
jupyter_v3), Logfire, Mailgun, New Relic, Okta, Plane, Supabase, Thrive, Tinybird, Twilio SMS and Twilio Voice (one shared Twilio credential), and Upstash.One check factory for every vendor.
tokenCheckValidatorgains:domainandworkspaceIdnormalizers: the normalized domain is what the credential stores and what runs use;egressprofile, which sends checks to a user-chosen host through the same SSRF-guarded egress the tools use (no redirects, size cap,ENOTFOUND→site_not_found);errorBodyEchoesToken, which keeps a vendor's token-echoing error body out of logs (Tinybird);acceptsBodycan name the credential from the verified reply.Shared
normalizeHttpBaseUrl,allowedDomainandrefuseUrlCredentialshelpers replace the per-vendor URL and region rules.GitLab moves onto it, so every token check is a short declaration in
validators/token-checks.ts.Every host-deciding param comes from the credential.
The connect form sends URL-shaped domains as typed. Instance and deployment URLs keep their scheme and path, and the server check normalizes them. Before this, it stripped them to a bare host, which broke Convex and dropped Jupyter and Grafana sub-paths.
Azure DevOps URLs are traversal-safe. Every project-scoped URL goes through one builder that runs the organization, project and ids through
safeUrlPathSegment, so a project like../../other-orgcan't carry a saved token into another organization. A legitimate name, including one with spaces or pasted already encoded, builds the same URL as before.Integration skills teach the pattern.
add-integration,add-block,add-toolsandvalidate-integrationnow describe token credentials (descriptor, check declaration, the shared policy mapping every host-deciding param) and credential-only successors for blocks that take a secret inline.New credentials default to a name with their domain or account ID (e.g. "Azure DevOps (contoso)"), and the audit log records it.
Databricks keyless tokens last up to an hour. Databricks gives its access token the same expiry as Sim's subject token, so long Genie polls no longer lose auth. The issuer caps any requested lifetime at an hour.
Release-audit fixes (credentials as a whole)
domain/instanceUrl/apiDomain/cloudIdfills no longer write a stored host into a same-named param. Before this, Thrive create/update user sent its region host as the user's domain.organization, Infisical base URL).ENOTFOUNDis read from the error or its cause, so a mistyped host or account id givessite_not_foundunder Bun too, not a retryable outage.Existing workflows don't change
hideFromToolbar: trueandsunset;jupyter_v2becomes legacy forjupyter_v3, andjupyter(v1) now points atjupyter_v3.oauth(required: false) on these tools, and Azure DevOps URLs (below).tokenCheckValidatorvalidators were run through 11 reply scenarios at both revisions: 814 cases, no differences.Held back
Each of these doesn't fit the pattern cleanly:
Type of Change
Testing
New tests:
site_not_found, no token in error details.Each guard was reverted to watch its test go red.
Targeted Vitest (99 files, 1,760 tests),
test:scripts,bun run lint,type-check,check-block-registry.ts origin/staging(incl. the saved-credential contract), and the canvas-sentences, docs, integration-catalog, deployment-config, block-successors, tool-metadata, boundaries, client-boundary, tool-registry-boundary, react-query, test-pattern, test-types, comment, utils, explicit-any, file-names, source-conventions and unused-exports audits.Parallel audits: per block (all 21), the factory and every existing validator, backwards compatibility (parity dump), and a coverage sweep of every block that still takes an inline secret. Fake keys were sent to each public vendor endpoint to confirm it rejects them.
Checklist
test-auditauthoring gate)