Expose AWS connector setup, create, and verify - #1780
Conversation
f675a01 to
221c4f2
Compare
221c4f2 to
48afb16
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| let cancelled = false; | ||
| setSetup(null); |
There was a problem hiding this comment.
🚫 [eslint (apps/console)] reported by reviewdog 🐶
Error: Calling setState synchronously within an effect can trigger cascading renders
48afb16 to
ae85b3e
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 59 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/server/api/console/v1/connector_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/connector_resolvers.go:230">
P2: When `connectorId` references an API-key or OAuth connector, this call returns an empty result and the mutation reports `ok: true`; reject non-workload-identity connectors before verification.</violation>
</file>
<file name="pkg/cloud/verify.go">
<violation number="1" location="pkg/cloud/verify.go:60">
P1: When a check has an unknown or zero status, `OK` treats it as passed and `Err` returns nil. Validate the three allowed statuses in both paths, or use a validated status type, so an incomplete grant cannot pass the activation gate.</violation>
</file>
<file name="pkg/cloud/verify_test.go">
<violation number="1" location="pkg/cloud/verify_test.go:47">
P3: The subtest name "OK when later checks were skipped" claims OK() is true, but the block asserts the opposite: `assert.False(t, result.OK())` and `require.Error(t, result.Err())`, because the first check fails and the skipped check after it does not affect the result. Rename it (e.g. "not OK when an earlier check failed even if later checks were skipped") so the name matches what the test verifies.</violation>
</file>
<file name="apps/console/src/pages/organizations/access-reviews/connections/_components/AccessReviewSourceProviderListItem.tsx">
<violation number="1" location="apps/console/src/pages/organizations/access-reviews/connections/_components/AccessReviewSourceProviderListItem.tsx:165">
P1: For AWS, `actions` is empty because `workloadIdentitySupported` is not included when building `methods`, so `ConnectMethodSplitButton` renders no connect control. Add the workload-identity method when this flag is true so the new dialog can be opened.</violation>
</file>
<file name="pkg/server/api/mcp/v1/specification.yaml">
<violation number="1" location="pkg/server/api/mcp/v1/specification.yaml:9845">
P2: This AWS-only tool accepts any provider string in its MCP schema, so clients can generate requests guaranteed to fail. Declare `provider` as an `AWS` enum.</violation>
<violation number="2" location="pkg/server/api/mcp/v1/specification.yaml:17525">
P2: Verification only reads the connector and AWS grant, so this hint incorrectly marks it as write-capable. Set `readonly` to true so MCP clients can treat it as a safe read.</violation>
</file>
<file name="pkg/cloud/aws/setup.go">
<violation number="1" location="pkg/cloud/aws/setup.go:202">
P1: When quick-create is configured, this emits a doubly encoded fragment, so AWS receives `templateURL` and Probo parameters containing percent-encoded text instead of their original values. Append the already encoded fragment to the URL string or set a matching raw fragment instead of assigning encoded query text to `URL.Fragment`.</violation>
</file>
<file name="pkg/cloud/aws/verify.go">
<violation number="1" location="pkg/cloud/aws/verify.go:36">
P2: When `GetRole` succeeds but its trust-policy document is malformed or undecodable, this message directs operators to troubleshoot IAM access instead of the policy. Include invalid trust-policy data in this message or classify parse failures separately.</violation>
</file>
<file name="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx">
<violation number="1" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:203">
P2: After verification succeeds, `createSourceAfterConnector` is still in flight when `finally` re-enables the submit button. A second click can run verification and source creation again before the first source mutation finishes; keep the working state until the source callback completes.</violation>
<violation number="2" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:219">
P2: When createWorkloadIdentityConnector or runVerify (which awaits verifyConnector) fails, the shared createUseMutation rejects the promise, so the rejection propagates out of this async connect(). There is no catch — only try/finally — and connect() is invoked as `void connect()`, producing an unhandled promise rejection in the browser on every connect/verify failure. The errorToast still surfaces a toast, so this is an error-handling gap rather than a missing UX message. Add a catch (and keep isWorking reset in finally) so rejected mutations are handled instead of relying on the calling frame.</violation>
<violation number="3" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:263">
P2: After a failed verification, editing the account ID or role name has no effect because retry always verifies the already-created connector. Disable these fields after creation or delete/recreate the connector when settings change so operators can correct setup mistakes.</violation>
<violation number="4" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:312">
P1: When verification fails and the operator closes the dialog, `resetForm` forgets the persisted connector without deleting it. The next AWS connection attempt then hits the one-connector-per-provider conflict; delete the failed connector on dismiss or retain a recoverable connector/retry path.</violation>
</file>
<file name="packages/n8n-node/nodes/Probo/actions/connector/create.operation.ts">
<violation number="1" location="packages/n8n-node/nodes/Probo/actions/connector/create.operation.ts:89">
P2: awsRoleName is trimmed before sending, but awsAccountId is passed through as-is even though the field is documented as a strict twelve-digit AWS account ID. A pasted ID containing whitespace or an invalid length will be sent unchanged and fail AWS matching. Normalize awsAccountId with trim() (and optionally validate the twelve-digit format) to match how awsRoleName is handled.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| // OK reports whether every check that ran passed. | ||
| func (r VerificationResult) OK() bool { | ||
| for _, check := range r.Checks { | ||
| if check.Status == VerificationStatusFailed { |
There was a problem hiding this comment.
P1: When a check has an unknown or zero status, OK treats it as passed and Err returns nil. Validate the three allowed statuses in both paths, or use a validated status type, so an incomplete grant cannot pass the activation gate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/verify.go, line 60:
<comment>When a check has an unknown or zero status, `OK` treats it as passed and `Err` returns nil. Validate the three allowed statuses in both paths, or use a validated status type, so an incomplete grant cannot pass the activation gate.</comment>
<file context>
@@ -0,0 +1,94 @@
+// OK reports whether every check that ran passed.
+func (r VerificationResult) OK() bool {
+ for _, check := range r.Checks {
+ if check.Status == VerificationStatusFailed {
+ return false
+ }
</file context>
| )} | ||
| /> | ||
| )} | ||
| <ConnectMethodSplitButton |
There was a problem hiding this comment.
P1: For AWS, actions is empty because workloadIdentitySupported is not included when building methods, so ConnectMethodSplitButton renders no connect control. Add the workload-identity method when this flag is true so the new dialog can be opened.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/connections/_components/AccessReviewSourceProviderListItem.tsx, line 165:
<comment>For AWS, `actions` is empty because `workloadIdentitySupported` is not included when building `methods`, so `ConnectMethodSplitButton` renders no connect control. Add the workload-identity method when this flag is true so the new dialog can be opened.</comment>
<file context>
@@ -155,20 +162,12 @@ export function AccessReviewSourceProviderListItem({
- )}
- />
- )}
+ <ConnectMethodSplitButton
+ actions={actions}
+ chooseAnotherMethodLabel={t(
</file context>
| params.Set("param_ProboIssuerURL", in.IssuerURL) | ||
| params.Set("param_ProboSubject", in.Subject) | ||
|
|
||
| u.Fragment = "/stacks/quickcreate?" + params.Encode() |
There was a problem hiding this comment.
P1: When quick-create is configured, this emits a doubly encoded fragment, so AWS receives templateURL and Probo parameters containing percent-encoded text instead of their original values. Append the already encoded fragment to the URL string or set a matching raw fragment instead of assigning encoded query text to URL.Fragment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/aws/setup.go, line 202:
<comment>When quick-create is configured, this emits a doubly encoded fragment, so AWS receives `templateURL` and Probo parameters containing percent-encoded text instead of their original values. Append the already encoded fragment to the URL string or set a matching raw fragment instead of assigning encoded query text to `URL.Fragment`.</comment>
<file context>
@@ -0,0 +1,205 @@
+ params.Set("param_ProboIssuerURL", in.IssuerURL)
+ params.Set("param_ProboSubject", in.Subject)
+
+ u.Fragment = "/stacks/quickcreate?" + params.Encode()
+
+ return u.String(), nil
</file context>
| return ( | ||
| <Dialog | ||
| ref={dialogRef} | ||
| onClose={() => { |
There was a problem hiding this comment.
P1: When verification fails and the operator closes the dialog, resetForm forgets the persisted connector without deleting it. The next AWS connection attempt then hits the one-connector-per-provider conflict; delete the failed connector on dismiss or retain a recoverable connector/retry path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 312:
<comment>When verification fails and the operator closes the dialog, `resetForm` forgets the persisted connector without deleting it. The next AWS connection attempt then hits the one-connector-per-provider conflict; delete the failed connector on dismiss or retain a recoverable connector/retry path.</comment>
<file context>
@@ -0,0 +1,478 @@
+ return (
+ <Dialog
+ ref={dialogRef}
+ onClose={() => {
+ resetForm();
+ onClose();
</file context>
| return nil, err | ||
| } | ||
|
|
||
| result, err := r.accessReview.InspectConnector(ctx, scope, input.ConnectorID) |
There was a problem hiding this comment.
P2: When connectorId references an API-key or OAuth connector, this call returns an empty result and the mutation reports ok: true; reject non-workload-identity connectors before verification.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/console/v1/connector_resolvers.go, line 230:
<comment>When `connectorId` references an API-key or OAuth connector, this call returns an empty result and the mutation reports `ok: true`; reject non-workload-identity connectors before verification.</comment>
<file context>
@@ -157,6 +159,90 @@ func (r *mutationResolver) CreateClientCredentialsConnector(ctx context.Context,
+ return nil, err
+ }
+
+ result, err := r.accessReview.InspectConnector(ctx, scope, input.ConnectorID)
+ if err != nil {
+ if errors.Is(err, coredata.ErrResourceNotFound) {
</file context>
| setSetupError(false); | ||
| setConnectorId(null); | ||
| setVerification(null); | ||
| setIsWorking(false); |
There was a problem hiding this comment.
P2: After verification succeeds, createSourceAfterConnector is still in flight when finally re-enables the submit button. A second click can run verification and source creation again before the first source mutation finishes; keep the working state until the source callback completes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 203:
<comment>After verification succeeds, `createSourceAfterConnector` is still in flight when `finally` re-enables the submit button. A second click can run verification and source creation again before the first source mutation finishes; keep the working state until the source callback completes.</comment>
<file context>
@@ -0,0 +1,478 @@
+ setSetupError(false);
+ setConnectorId(null);
+ setVerification(null);
+ setIsWorking(false);
+ };
+
</file context>
|
|
||
| try { | ||
| let id = connectorId; | ||
| if (!id) { |
There was a problem hiding this comment.
P2: After a failed verification, editing the account ID or role name has no effect because retry always verifies the already-created connector. Disable these fields after creation or delete/recreate the connector when settings change so operators can correct setup mistakes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 263:
<comment>After a failed verification, editing the account ID or role name has no effect because retry always verifies the already-created connector. Disable these fields after creation or delete/recreate the connector when settings change so operators can correct setup mistakes.</comment>
<file context>
@@ -0,0 +1,478 @@
+
+ try {
+ let id = connectorId;
+ if (!id) {
+ const extraFields = buildExtraFields(
+ provider.provider,
</file context>
| ): Promise<INodeExecutionData> { | ||
| const organizationId = this.getNodeParameter('organizationId', itemIndex) as string; | ||
| const provider = this.getNodeParameter('provider', itemIndex) as string; | ||
| const awsAccountId = this.getNodeParameter('awsAccountId', itemIndex) as string; |
There was a problem hiding this comment.
P2: awsRoleName is trimmed before sending, but awsAccountId is passed through as-is even though the field is documented as a strict twelve-digit AWS account ID. A pasted ID containing whitespace or an invalid length will be sent unchanged and fail AWS matching. Normalize awsAccountId with trim() (and optionally validate the twelve-digit format) to match how awsRoleName is handled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/n8n-node/nodes/Probo/actions/connector/create.operation.ts, line 89:
<comment>awsRoleName is trimmed before sending, but awsAccountId is passed through as-is even though the field is documented as a strict twelve-digit AWS account ID. A pasted ID containing whitespace or an invalid length will be sent unchanged and fail AWS matching. Normalize awsAccountId with trim() (and optionally validate the twelve-digit format) to match how awsRoleName is handled.</comment>
<file context>
@@ -0,0 +1,120 @@
+): Promise<INodeExecutionData> {
+ const organizationId = this.getNodeParameter('organizationId', itemIndex) as string;
+ const provider = this.getNodeParameter('provider', itemIndex) as string;
+ const awsAccountId = this.getNodeParameter('awsAccountId', itemIndex) as string;
+ const awsRoleName = this.getNodeParameter('awsRoleName', itemIndex, '') as string;
+
</file context>
| const awsAccountId = this.getNodeParameter('awsAccountId', itemIndex) as string; | |
| const awsAccountId = (this.getNodeParameter('awsAccountId', itemIndex) as string).trim(); |
| return; | ||
| } | ||
|
|
||
| try { |
There was a problem hiding this comment.
P2: When createWorkloadIdentityConnector or runVerify (which awaits verifyConnector) fails, the shared createUseMutation rejects the promise, so the rejection propagates out of this async connect(). There is no catch — only try/finally — and connect() is invoked as void connect(), producing an unhandled promise rejection in the browser on every connect/verify failure. The errorToast still surfaces a toast, so this is an error-handling gap rather than a missing UX message. Add a catch (and keep isWorking reset in finally) so rejected mutations are handled instead of relying on the calling frame.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 219:
<comment>When createWorkloadIdentityConnector or runVerify (which awaits verifyConnector) fails, the shared createUseMutation rejects the promise, so the rejection propagates out of this async connect(). There is no catch — only try/finally — and connect() is invoked as `void connect()`, producing an unhandled promise rejection in the browser on every connect/verify failure. The errorToast still surfaces a toast, so this is an error-handling gap rather than a missing UX message. Add a catch (and keep isWorking reset in finally) so rejected mutations are handled instead of relying on the calling frame.</comment>
<file context>
@@ -0,0 +1,478 @@
+ return;
+ }
+
+ try {
+ navigator.clipboard.writeText(value).then(
+ () =>
</file context>
| assert.NoError(t, result.Err()) | ||
| }) | ||
|
|
||
| t.Run("OK when later checks were skipped", func(t *testing.T) { |
There was a problem hiding this comment.
P3: The subtest name "OK when later checks were skipped" claims OK() is true, but the block asserts the opposite: assert.False(t, result.OK()) and require.Error(t, result.Err()), because the first check fails and the skipped check after it does not affect the result. Rename it (e.g. "not OK when an earlier check failed even if later checks were skipped") so the name matches what the test verifies.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/verify_test.go, line 47:
<comment>The subtest name "OK when later checks were skipped" claims OK() is true, but the block asserts the opposite: `assert.False(t, result.OK())` and `require.Error(t, result.Err())`, because the first check fails and the skipped check after it does not affect the result. Rename it (e.g. "not OK when an earlier check failed even if later checks were skipped") so the name matches what the test verifies.</comment>
<file context>
@@ -0,0 +1,74 @@
+ assert.NoError(t, result.Err())
+ })
+
+ t.Run("OK when later checks were skipped", func(t *testing.T) {
+ t.Parallel()
+
</file context>
| t.Run("OK when later checks were skipped", func(t *testing.T) { | |
| t.Run("not OK when an earlier check failed even if later checks were skipped", func(t *testing.T) { |
ae85b3e to
303da65
Compare
303da65 to
5f33678
Compare
There was a problem hiding this comment.
6 existing issues remain and 4 new issues found across 60 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/cloud/aws/trustpolicy.go">
<violation number="1" location="pkg/cloud/aws/trustpolicy.go:293">
P3: When a trust policy uses a non-pattern operator such as `StringNotEquals` or `ArnEquals`, this classification makes the verification check claim that the condition admits patterns. Add a distinct unsupported-operator reason and message, or classify only genuinely pattern-based operators as `TrustPolicyReasonPattern`.</violation>
</file>
<file name="e2e/mcp/aws_connector_test.go">
<violation number="1" location="e2e/mcp/aws_connector_test.go:88">
P2: This test couples CI to live outbound AWS: verifyConnector performs a real STS AssumeRoleWithWebIdentity for the fabricated 123456789012 account, and the assertions (Ok false, ASSUME_ROLE FAILED, message non-empty) depend on AWS responding with an AccessDenied/missing-role error. In an e2e sandbox without AWS egress the STS call can time out, error at session build, or return a different failure shape (and the MCP client caps at a 30s request timeout), making the test environment-sensitive and slow. Consider driving the failure deterministically with an in-process AWS endpoint (e.g., a mock STS responding AccessDenied for the fixture role) instead of relying on live AWS to prove the redaction and check-reporting behavior.</violation>
</file>
<file name="pkg/connector/provider/aws.go">
<violation number="1" location="pkg/connector/provider/aws.go:142">
P2: The ACCOUNT_MATCH check can never fail. verifyAWS passes AccountID from awsSession.AccountID(), which is parsed from the same role ARN the session assumes, so callerIdentity's account always equals it. To make the check meaningful, supply the account ID captured from the customer's independent setup input (the accountId ExtraSetting) rather than the ARN-derived value; otherwise the check is dead and gives false assurance.</violation>
</file>
<file name="pkg/server/api/console/v1/connector_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/connector_resolvers.go:225">
P2: VerifyConnector gates verification behind ActionConnectorCreate even though it is a read-only inspection of an existing connector. Use a read action such as ActionConnectorGet so verification is not tied to the create permission, matching how DeleteConnector uses ActionConnectorDelete for its operation.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 6 unresolved issues already reported by Cubic.
Fix all with cubic | Re-trigger cubic
| } `json:"checks"` | ||
| } `json:"verification"` | ||
| } | ||
| mc.CallToolInto("verifyConnector", map[string]any{ |
There was a problem hiding this comment.
P2: This test couples CI to live outbound AWS: verifyConnector performs a real STS AssumeRoleWithWebIdentity for the fabricated 123456789012 account, and the assertions (Ok false, ASSUME_ROLE FAILED, message non-empty) depend on AWS responding with an AccessDenied/missing-role error. In an e2e sandbox without AWS egress the STS call can time out, error at session build, or return a different failure shape (and the MCP client caps at a 30s request timeout), making the test environment-sensitive and slow. Consider driving the failure deterministically with an in-process AWS endpoint (e.g., a mock STS responding AccessDenied for the fixture role) instead of relying on live AWS to prove the redaction and check-reporting behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/mcp/aws_connector_test.go, line 88:
<comment>This test couples CI to live outbound AWS: verifyConnector performs a real STS AssumeRoleWithWebIdentity for the fabricated 123456789012 account, and the assertions (Ok false, ASSUME_ROLE FAILED, message non-empty) depend on AWS responding with an AccessDenied/missing-role error. In an e2e sandbox without AWS egress the STS call can time out, error at session build, or return a different failure shape (and the MCP client caps at a 30s request timeout), making the test environment-sensitive and slow. Consider driving the failure deterministically with an in-process AWS endpoint (e.g., a mock STS responding AccessDenied for the fixture role) instead of relying on live AWS to prove the redaction and check-reporting behavior.</comment>
<file context>
@@ -0,0 +1,169 @@
+ } `json:"checks"`
+ } `json:"verification"`
+ }
+ mc.CallToolInto("verifyConnector", map[string]any{
+ "connector_id": createResult.Connector.ID,
+ }, &verifyResult)
</file context>
| ctx, | ||
| cloudaws.VerifyAuditRoleInput{ | ||
| RoleName: roleName, | ||
| AccountID: awsSession.AccountID(), |
There was a problem hiding this comment.
P2: The ACCOUNT_MATCH check can never fail. verifyAWS passes AccountID from awsSession.AccountID(), which is parsed from the same role ARN the session assumes, so callerIdentity's account always equals it. To make the check meaningful, supply the account ID captured from the customer's independent setup input (the accountId ExtraSetting) rather than the ARN-derived value; otherwise the check is dead and gives false assurance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/connector/provider/aws.go, line 142:
<comment>The ACCOUNT_MATCH check can never fail. verifyAWS passes AccountID from awsSession.AccountID(), which is parsed from the same role ARN the session assumes, so callerIdentity's account always equals it. To make the check meaningful, supply the account ID captured from the customer's independent setup input (the accountId ExtraSetting) rather than the ARN-derived value; otherwise the check is dead and gives false assurance.</comment>
<file context>
@@ -111,27 +115,37 @@ func probeAWS(ctx context.Context, session cloud.Session, _ *coredata.Connector)
ctx,
cloudaws.VerifyAuditRoleInput{
RoleName: roleName,
+ AccountID: awsSession.AccountID(),
FederatedIdentity: awsSession.Identity(),
},
</file context>
|
|
||
| // VerifyConnector is the resolver for the verifyConnector field. | ||
| func (r *mutationResolver) VerifyConnector(ctx context.Context, input types.VerifyConnectorInput) (*types.VerifyConnectorPayload, error) { | ||
| scope, err := r.authorize(ctx, input.ConnectorID, probo.ActionConnectorCreate) |
There was a problem hiding this comment.
P2: VerifyConnector gates verification behind ActionConnectorCreate even though it is a read-only inspection of an existing connector. Use a read action such as ActionConnectorGet so verification is not tied to the create permission, matching how DeleteConnector uses ActionConnectorDelete for its operation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/console/v1/connector_resolvers.go, line 225:
<comment>VerifyConnector gates verification behind ActionConnectorCreate even though it is a read-only inspection of an existing connector. Use a read action such as ActionConnectorGet so verification is not tied to the create permission, matching how DeleteConnector uses ActionConnectorDelete for its operation.</comment>
<file context>
@@ -157,6 +159,90 @@ func (r *mutationResolver) CreateClientCredentialsConnector(ctx context.Context,
+
+// VerifyConnector is the resolver for the verifyConnector field.
+func (r *mutationResolver) VerifyConnector(ctx context.Context, input types.VerifyConnectorInput) (*types.VerifyConnectorPayload, error) {
+ scope, err := r.authorize(ctx, input.ConnectorID, probo.ActionConnectorCreate)
+ if err != nil {
+ return nil, err
</file context>
| scope, err := r.authorize(ctx, input.ConnectorID, probo.ActionConnectorCreate) | |
| scope, err := r.authorize(ctx, input.ConnectorID, probo.ActionConnectorGet) |
| conditionStringEquals, | ||
| return newTrustPolicyClaimError( | ||
| suffix, | ||
| TrustPolicyReasonPattern, |
There was a problem hiding this comment.
P3: When a trust policy uses a non-pattern operator such as StringNotEquals or ArnEquals, this classification makes the verification check claim that the condition admits patterns. Add a distinct unsupported-operator reason and message, or classify only genuinely pattern-based operators as TrustPolicyReasonPattern.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/aws/trustpolicy.go, line 293:
<comment>When a trust policy uses a non-pattern operator such as `StringNotEquals` or `ArnEquals`, this classification makes the verification check claim that the condition admits patterns. Add a distinct unsupported-operator reason and message, or classify only genuinely pattern-based operators as `TrustPolicyReasonPattern`.</comment>
<file context>
@@ -246,28 +288,40 @@ func (s TrustStatement) verifyClaim(hostPath, suffix, want string) error {
- conditionStringEquals,
+ return newTrustPolicyClaimError(
+ suffix,
+ TrustPolicyReasonPattern,
+ fmt.Errorf(
+ "cannot verify role trust policy: condition %q uses %s, which admits patterns; it must use %s",
</file context>
5f33678 to
2f8648e
Compare
There was a problem hiding this comment.
3 existing issues remain and 6 new issues found across 58 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/cmd/connector/create/create.go">
<violation number="1" location="pkg/cmd/connector/create/create.go:126">
P2: Passing any provider other than AWS makes this workload-identity command send a mutation the resolver always rejects. Remove `--provider` and hard-code AWS, or validate the flag locally as AWS.</violation>
</file>
<file name="pkg/cmd/connector/verify/verify.go">
<violation number="1" location="pkg/cmd/connector/verify/verify.go:99">
P1: When a connector check fails, this command still exits successfully because it ignores `verification.ok`. Return an error after printing the result when `Ok` is false so CI and scripts cannot treat an unusable connector as verified.</violation>
</file>
<file name="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx">
<violation number="1" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:287">
P2: When the dialog closes while connector creation or verification is in flight, the async continuation still creates the source or restores stale state. Ignore completions after close, or cancel the in-flight operation before updating state and creating the source.</violation>
<violation number="2" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:409">
P3: The three copy buttons for issuer, audience, and subject all use the same aria-label ("Copy"), so assistive-technology users cannot distinguish which value each copies. Include the field name in the accessible label for each row.</violation>
</file>
<file name="pkg/server/api/mcp/v1/schema.resolvers.go">
<violation number="1" location="pkg/server/api/mcp/v1/schema.resolvers.go:9401">
P2: When `connectorId` refers to a non-workload-identity connector, this endpoint reports verification as successful with no checks. Restrict this operation to AWS workload-identity connectors or return an explicit unsupported-connector result instead of treating the empty result as success.</violation>
</file>
<file name="packages/n8n-node/nodes/Probo/actions/connector/create.operation.ts">
<violation number="1" location="packages/n8n-node/nodes/Probo/actions/connector/create.operation.ts:110">
P3: In the create connector execute, `awsRoleName` is trimmed before being added to the input, but `organizationId` and `awsAccountId` are sent untrimmed. Apply the same `.trim()` to the other string inputs so all passed values are sanitized consistently.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return fmt.Errorf("cannot parse response: %w", err) | ||
| } | ||
|
|
||
| return cmdutil.PrintJSON(f.IOStreams.Out, resp.VerifyConnector.Verification) |
There was a problem hiding this comment.
P1: When a connector check fails, this command still exits successfully because it ignores verification.ok. Return an error after printing the result when Ok is false so CI and scripts cannot treat an unusable connector as verified.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/connector/verify/verify.go, line 99:
<comment>When a connector check fails, this command still exits successfully because it ignores `verification.ok`. Return an error after printing the result when `Ok` is false so CI and scripts cannot treat an unusable connector as verified.</comment>
<file context>
@@ -0,0 +1,107 @@
+ return fmt.Errorf("cannot parse response: %w", err)
+ }
+
+ return cmdutil.PrintJSON(f.IOStreams.Out, resp.VerifyConnector.Verification)
+ },
+ }
</file context>
| return cmdutil.PrintJSON(f.IOStreams.Out, resp.VerifyConnector.Verification) | |
| if err := cmdutil.PrintJSON(f.IOStreams.Out, resp.VerifyConnector.Verification); err != nil { | |
| return err | |
| } | |
| if !resp.VerifyConnector.Verification.Ok { | |
| return fmt.Errorf("connector verification failed") | |
| } | |
| return nil |
| } | ||
|
|
||
| cmd.Flags().StringVar(&flagOrg, "org", "", "Organization ID") | ||
| cmd.Flags().StringVar(&flagProvider, "provider", "AWS", "Connector provider") |
There was a problem hiding this comment.
P2: Passing any provider other than AWS makes this workload-identity command send a mutation the resolver always rejects. Remove --provider and hard-code AWS, or validate the flag locally as AWS.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/connector/create/create.go, line 126:
<comment>Passing any provider other than AWS makes this workload-identity command send a mutation the resolver always rejects. Remove `--provider` and hard-code AWS, or validate the flag locally as AWS.</comment>
<file context>
@@ -0,0 +1,132 @@
+ }
+
+ cmd.Flags().StringVar(&flagOrg, "org", "", "Organization ID")
+ cmd.Flags().StringVar(&flagProvider, "provider", "AWS", "Connector provider")
+ cmd.Flags().StringVar(&flagAccountID, "aws-account-id", "", "Twelve-digit AWS account ID")
+ cmd.Flags().StringVar(&flagRoleName, "aws-role-name", "", "IAM role name (default ProboAudit)")
</file context>
| setConnectorId(id); | ||
| } | ||
|
|
||
| const next = await runVerify(id); |
There was a problem hiding this comment.
P2: When the dialog closes while connector creation or verification is in flight, the async continuation still creates the source or restores stale state. Ignore completions after close, or cancel the in-flight operation before updating state and creating the source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 287:
<comment>When the dialog closes while connector creation or verification is in flight, the async continuation still creates the source or restores stale state. Ignore completions after close, or cancel the in-flight operation before updating state and creating the source.</comment>
<file context>
@@ -0,0 +1,478 @@
+ setConnectorId(id);
+ }
+
+ const next = await runVerify(id);
+ if (next.ok) {
+ createSourceAfterConnector(id, provider.displayName, () => {
</file context>
| return nil, types.VerifyConnectorOutput{}, err | ||
| } | ||
|
|
||
| result, err := r.accessReview.InspectConnector(ctx, scope, input.ConnectorID) |
There was a problem hiding this comment.
P2: When connectorId refers to a non-workload-identity connector, this endpoint reports verification as successful with no checks. Restrict this operation to AWS workload-identity connectors or return an explicit unsupported-connector result instead of treating the empty result as success.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/mcp/v1/schema.resolvers.go, line 9401:
<comment>When `connectorId` refers to a non-workload-identity connector, this endpoint reports verification as successful with no checks. Restrict this operation to AWS workload-identity connectors or return an explicit unsupported-connector result instead of treating the empty result as success.</comment>
<file context>
@@ -9296,3 +9298,118 @@ func mapTreatmentPlanError(ctx context.Context, logger *log.Logger, op string, e
+ return nil, types.VerifyConnectorOutput{}, err
+ }
+
+ result, err := r.accessReview.InspectConnector(ctx, scope, input.ConnectorID)
+ if err != nil {
+ if errors.Is(err, coredata.ErrResourceNotFound) {
</file context>
| variant="secondary" | ||
| icon={IconSquareBehindSquare2} | ||
| onClick={() => copyValue(row.value, row.successKey)} | ||
| aria-label={t( |
There was a problem hiding this comment.
P3: The three copy buttons for issuer, audience, and subject all use the same aria-label ("Copy"), so assistive-technology users cannot distinguish which value each copies. Include the field name in the accessible label for each row.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 409:
<comment>The three copy buttons for issuer, audience, and subject all use the same aria-label ("Copy"), so assistive-technology users cannot distinguish which value each copies. Include the field name in the accessible label for each row.</comment>
<file context>
@@ -0,0 +1,478 @@
+ variant="secondary"
+ icon={IconSquareBehindSquare2}
+ onClick={() => copyValue(row.value, row.successKey)}
+ aria-label={t(
+ "workloadIdentityConnectorDialog.actions.copy",
+ )}
</file context>
| provider, | ||
| awsAccountId, | ||
| }; | ||
| if (awsRoleName.trim()) { |
There was a problem hiding this comment.
P3: In the create connector execute, awsRoleName is trimmed before being added to the input, but organizationId and awsAccountId are sent untrimmed. Apply the same .trim() to the other string inputs so all passed values are sanitized consistently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/n8n-node/nodes/Probo/actions/connector/create.operation.ts, line 110:
<comment>In the create connector execute, `awsRoleName` is trimmed before being added to the input, but `organizationId` and `awsAccountId` are sent untrimmed. Apply the same `.trim()` to the other string inputs so all passed values are sanitized consistently.</comment>
<file context>
@@ -0,0 +1,120 @@
+ provider,
+ awsAccountId,
+ };
+ if (awsRoleName.trim()) {
+ input.awsRoleName = awsRoleName.trim();
+ }
</file context>
2f8648e to
802b031
Compare
802b031 to
3f523ec
Compare
There was a problem hiding this comment.
6 existing issues remain and 5 new issues found across 58 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/console/src/pages/organizations/access-reviews/connections/_components/AccessReviewSourceProviderListItem.tsx">
<violation number="1" location="apps/console/src/pages/organizations/access-reviews/connections/_components/AccessReviewSourceProviderListItem.tsx:204">
P2: When a non-AWS provider advertises `workloadIdentitySupported`, this row opens an AWS-only dialog and submits AWS-specific setup fields. Render a provider-specific dialog, or restrict both the action and dialog to AWS until other workload-identity implementations exist.</violation>
</file>
<file name="pkg/server/api/console/v1/connector_workload_identity.go">
<violation number="1" location="pkg/server/api/console/v1/connector_workload_identity.go:42">
P3: These console adapters duplicate the existing MCP setup and verification mappers, so future fields or enum changes can update one API surface while leaving the other stale. Move the shared cloud-to-API mapping into a common helper or shared intermediate representation.</violation>
</file>
<file name="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx">
<violation number="1" location="apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx:362">
P2: When setup loading fails, this only displays “Try again” without offering a retry; the query runs again only after closing and reopening the dialog. Add a retry control or retry state for transient setup failures.</violation>
</file>
<file name="pkg/cloud/verify.go">
<violation number="1" location="pkg/cloud/verify.go:28">
P2: The public verification API accepts arbitrary strings for codes and statuses, so a typo or zero value can make `OK()` return true and produce a value outside the GraphQL enums. Define typed `VerificationStatus` and `VerificationCheckCode` values, use them on `VerificationCheck` and `Set`, and validate any externally constructed result before reporting success.</violation>
</file>
<file name="pkg/server/api/mcp/v1/specification.yaml">
<violation number="1" location="pkg/server/api/mcp/v1/specification.yaml:9844">
P2: CreateWorkloadIdentityConnectorMCPInput makes `provider` required but leaves it an unconstrained string, so any arbitrary value (e.g. "azure", "gcp") passes MCP validation and must be rejected in the resolver. Since this PR only supports AWS, constrain the field with the existing `ConnectorProvider` enum (or a fixed-string pattern) in the input schema so invalid providers are rejected at the contract boundary. Same applies to `aws_account_id`, which is documented as a twelve-digit AWS account ID but has no pattern/maxLength check.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 6 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| onClose={() => setActiveDialog(null)} | ||
| /> | ||
| )} | ||
| {provider.workloadIdentitySupported && ( |
There was a problem hiding this comment.
P2: When a non-AWS provider advertises workloadIdentitySupported, this row opens an AWS-only dialog and submits AWS-specific setup fields. Render a provider-specific dialog, or restrict both the action and dialog to AWS until other workload-identity implementations exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/connections/_components/AccessReviewSourceProviderListItem.tsx, line 204:
<comment>When a non-AWS provider advertises `workloadIdentitySupported`, this row opens an AWS-only dialog and submits AWS-specific setup fields. Render a provider-specific dialog, or restrict both the action and dialog to AWS until other workload-identity implementations exist.</comment>
<file context>
@@ -202,6 +201,15 @@ export function AccessReviewSourceProviderListItem({
onClose={() => setActiveDialog(null)}
/>
)}
+ {provider.workloadIdentitySupported && (
+ <WorkloadIdentityConnectorDialog
+ providerKey={activeDialog === "workloadIdentity" ? provider : null}
</file context>
| })} | ||
| {setupError && ( | ||
| <p className="text-sm text-txt-danger"> | ||
| {t("workloadIdentityConnectorDialog.errors.setup")} |
There was a problem hiding this comment.
P2: When setup loading fails, this only displays “Try again” without offering a retry; the query runs again only after closing and reopening the dialog. Add a retry control or retry state for transient setup failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/access-reviews/dialogs/_components/WorkloadIdentityConnectorDialog.tsx, line 362:
<comment>When setup loading fails, this only displays “Try again” without offering a retry; the query runs again only after closing and reopening the dialog. Add a retry control or retry state for transient setup failures.</comment>
<file context>
@@ -0,0 +1,478 @@
+ })}
+ {setupError && (
+ <p className="text-sm text-txt-danger">
+ {t("workloadIdentityConnectorDialog.errors.setup")}
+ </p>
+ )}
</file context>
| const ( | ||
| // Verification statuses are the three-valued outcome of one grant check. | ||
| // SKIPPED means an earlier check failed, so this one never ran. | ||
| VerificationStatusPassed = "PASSED" |
There was a problem hiding this comment.
P2: The public verification API accepts arbitrary strings for codes and statuses, so a typo or zero value can make OK() return true and produce a value outside the GraphQL enums. Define typed VerificationStatus and VerificationCheckCode values, use them on VerificationCheck and Set, and validate any externally constructed result before reporting success.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/verify.go, line 28:
<comment>The public verification API accepts arbitrary strings for codes and statuses, so a typo or zero value can make `OK()` return true and produce a value outside the GraphQL enums. Define typed `VerificationStatus` and `VerificationCheckCode` values, use them on `VerificationCheck` and `Set`, and validate any externally constructed result before reporting success.</comment>
<file context>
@@ -0,0 +1,92 @@
+const (
+ // Verification statuses are the three-valued outcome of one grant check.
+ // SKIPPED means an earlier check failed, so this one never ran.
+ VerificationStatusPassed = "PASSED"
+ VerificationStatusFailed = "FAILED"
+ VerificationStatusSkipped = "SKIPPED"
</file context>
| id: | ||
| $ref: "#/components/schemas/GID" | ||
| description: Connector ID | ||
| provider: |
There was a problem hiding this comment.
P2: CreateWorkloadIdentityConnectorMCPInput makes provider required but leaves it an unconstrained string, so any arbitrary value (e.g. "azure", "gcp") passes MCP validation and must be rejected in the resolver. Since this PR only supports AWS, constrain the field with the existing ConnectorProvider enum (or a fixed-string pattern) in the input schema so invalid providers are rejected at the contract boundary. Same applies to aws_account_id, which is documented as a twelve-digit AWS account ID but has no pattern/maxLength check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/mcp/v1/specification.yaml, line 9844:
<comment>CreateWorkloadIdentityConnectorMCPInput makes `provider` required but leaves it an unconstrained string, so any arbitrary value (e.g. "azure", "gcp") passes MCP validation and must be rejected in the resolver. Since this PR only supports AWS, constrain the field with the existing `ConnectorProvider` enum (or a fixed-string pattern) in the input schema so invalid providers are rejected at the contract boundary. Same applies to `aws_account_id`, which is documented as a twelve-digit AWS account ID but has no pattern/maxLength check.</comment>
<file context>
@@ -9830,6 +9830,168 @@ components:
+ id:
+ $ref: "#/components/schemas/GID"
+ description: Connector ID
+ provider:
+ type: string
+ description: Connector provider
</file context>
| _, _ = w.Write(cloudformation.AuditRoleTemplate) | ||
| } | ||
|
|
||
| func newAWSConnectorSetup(setup cloudaws.ConnectorSetup) *types.AWSConnectorSetup { |
There was a problem hiding this comment.
P3: These console adapters duplicate the existing MCP setup and verification mappers, so future fields or enum changes can update one API surface while leaving the other stale. Move the shared cloud-to-API mapping into a common helper or shared intermediate representation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/console/v1/connector_workload_identity.go, line 42:
<comment>These console adapters duplicate the existing MCP setup and verification mappers, so future fields or enum changes can update one API surface while leaving the other stale. Move the shared cloud-to-API mapping into a common helper or shared intermediate representation.</comment>
<file context>
@@ -0,0 +1,80 @@
+ _, _ = w.Write(cloudformation.AuditRoleTemplate)
+}
+
+func newAWSConnectorSetup(setup cloudaws.ConnectorSetup) *types.AWSConnectorSetup {
+ out := &types.AWSConnectorSetup{
+ Issuer: setup.Issuer,
</file context>
3d5fadc to
6bd3124
Compare
There was a problem hiding this comment.
6 existing issues remain and 3 new issues found across 49 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/cloud/aws/setup.go">
<violation number="1" location="pkg/cloud/aws/setup.go:36">
P2: The generated Terraform setup is not reproducible because this GitHub module source is unpinned. Pin it to a published `terraform-aws-audit-role/v...` release so future module changes cannot silently alter the IAM resources or permissions an existing setup applies.</violation>
<violation number="2" location="pkg/cloud/aws/setup.go:128">
P1: When the customer deploys the setup module in GovCloud or China, this constructor stores a commercial-partition ARN even though the module creates a partition-specific ARN. Persist the selected AWS partition and build the connector ARN from it, or reject unsupported partitions before creating the connector.</violation>
</file>
<file name="pkg/server/api/console/v1/graphql/connector.graphql">
<violation number="1" location="pkg/server/api/console/v1/graphql/connector.graphql:195">
P2: When identity federation is disabled, `accessReviewDrivers` still reports AWS `workloadIdentitySupported: true`, so clients show a connect form that the setup and create operations reject. Populate this field only when `r.identityFederation != nil` as well as when the provider supports workload identity.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 6 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| } | ||
|
|
||
| return coredata.AWSConnectorSettings{ | ||
| RoleARN: arn.RoleARN(accountID, roleName), |
There was a problem hiding this comment.
P1: When the customer deploys the setup module in GovCloud or China, this constructor stores a commercial-partition ARN even though the module creates a partition-specific ARN. Persist the selected AWS partition and build the connector ARN from it, or reject unsupported partitions before creating the connector.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/aws/setup.go, line 128:
<comment>When the customer deploys the setup module in GovCloud or China, this constructor stores a commercial-partition ARN even though the module creates a partition-specific ARN. Persist the selected AWS partition and build the connector ARN from it, or reject unsupported partitions before creating the connector.</comment>
<file context>
@@ -0,0 +1,148 @@
+ }
+
+ return coredata.AWSConnectorSettings{
+ RoleARN: arn.RoleARN(accountID, roleName),
+ Issuer: issuer,
+ }, nil
</file context>
| ) | ||
|
|
||
| const ( | ||
| terraformModuleSource = "github.com/getprobo/probo//contrib/terraform/aws-audit-role" |
There was a problem hiding this comment.
P2: The generated Terraform setup is not reproducible because this GitHub module source is unpinned. Pin it to a published terraform-aws-audit-role/v... release so future module changes cannot silently alter the IAM resources or permissions an existing setup applies.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/aws/setup.go, line 36:
<comment>The generated Terraform setup is not reproducible because this GitHub module source is unpinned. Pin it to a published `terraform-aws-audit-role/v...` release so future module changes cannot silently alter the IAM resources or permissions an existing setup applies.</comment>
<file context>
@@ -0,0 +1,148 @@
+)
+
+const (
+ terraformModuleSource = "github.com/getprobo/probo//contrib/terraform/aws-audit-role"
+)
+
</file context>
| terraformModuleSource = "github.com/getprobo/probo//contrib/terraform/aws-audit-role" | |
| terraformModuleSource = "github.com/getprobo/probo//contrib/terraform/aws-audit-role?ref=terraform-aws-audit-role/v0.1.0" |
| a Probo-issued OIDC token into the customer's cloud account, and this | ||
| deployment has the identity federation issuer enabled. | ||
| """ | ||
| workloadIdentitySupported: Boolean! |
There was a problem hiding this comment.
P2: When identity federation is disabled, accessReviewDrivers still reports AWS workloadIdentitySupported: true, so clients show a connect form that the setup and create operations reject. Populate this field only when r.identityFederation != nil as well as when the provider supports workload identity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/console/v1/graphql/connector.graphql, line 195:
<comment>When identity federation is disabled, `accessReviewDrivers` still reports AWS `workloadIdentitySupported: true`, so clients show a connect form that the setup and create operations reject. Populate this field only when `r.identityFederation != nil` as well as when the provider supports workload identity.</comment>
<file context>
@@ -187,6 +187,48 @@ type ConnectorProviderInfo {
+ a Probo-issued OIDC token into the customer's cloud account, and this
+ deployment has the identity federation issuer enabled.
+ """
+ workloadIdentitySupported: Boolean!
+ """
+ workloadIdentityExtraSettings lists the settings the workload-identity
</file context>
The access-review driver can assume an audit role, but operators had no supported path to create that connector. Add GraphQL, MCP, CLI, and n8n operations plus a console dialog so a single AWS account can be connected with server-built setup artifacts and named verification checks. Signed-off-by: Ludovic Vielle <ludovic@probo.com>
6bd3124 to
4e6bf9d
Compare
There was a problem hiding this comment.
7 existing issues remain and 1 new issue found across 49 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/cloud/verify_test.go">
<violation number="1" location="pkg/cloud/verify_test.go:47">
P3: The subtest name "OK when later checks were skipped" contradicts its own assertion: it contains a FAILED check and asserts result.OK() is false. The behavior it names — skipped checks not making OK false — is only exercised when no check failed, and that case has no coverage. Rename the subtest to reflect the failure, or add a separate case with all checks SKIPPED asserting OK() is true.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 7 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| assert.NoError(t, result.Err()) | ||
| }) | ||
|
|
||
| t.Run("OK when later checks were skipped", func(t *testing.T) { |
There was a problem hiding this comment.
P3: The subtest name "OK when later checks were skipped" contradicts its own assertion: it contains a FAILED check and asserts result.OK() is false. The behavior it names — skipped checks not making OK false — is only exercised when no check failed, and that case has no coverage. Rename the subtest to reflect the failure, or add a separate case with all checks SKIPPED asserting OK() is true.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cloud/verify_test.go, line 47:
<comment>The subtest name "OK when later checks were skipped" contradicts its own assertion: it contains a FAILED check and asserts result.OK() is false. The behavior it names — skipped checks not making OK false — is only exercised when no check failed, and that case has no coverage. Rename the subtest to reflect the failure, or add a separate case with all checks SKIPPED asserting OK() is true.</comment>
<file context>
@@ -0,0 +1,74 @@
+ assert.NoError(t, result.Err())
+ })
+
+ t.Run("OK when later checks were skipped", func(t *testing.T) {
+ t.Parallel()
+
</file context>
1 participant
The access-review driver can assume an audit role, but operators had no supported path to create that connector. Add GraphQL, MCP, CLI, and n8n operations plus a console dialog so a single AWS account can be connected with server-built setup artifacts and named verification checks.
Summary by cubic
Operators previously had no supported way to create the AWS audit-role connector, even though access reviews could assume the role. This adds setup, creation, and verification flows across the API, CLI, n8n, MCP, and console, with server-generated federation artifacts and named checks. AWS quick-create remains optional and requires a configured public CloudFormation template URL.
Tests
+925-11Covers AWS setup artifacts, connector creation and verification, API behavior, and provider capability discovery across end-to-end and unit tests.
GraphQL API
+367-52Adds AWS setup queries, workload-identity connector creation and verification, provider settings, and federation availability checks.
MCP
+419-16Adds setup, create, and verify tools with connector types and OpenAPI schema definitions.
prb (CLI)
+392-0Adds
prb connector setup-aws,prb connector create, andprb connector verifycommands.Service
+644-67Generates AWS setup artifacts, validates account and role configuration, and returns named checks for role assumption and trust-policy configuration.
App: console
+651-54Adds a localized workload-identity dialog that collects AWS settings, displays federation values and setup artifacts, and reports verification results.
Package: n8n-node
+328-0Adds a Connector resource with
setupAws,create, andverifyoperations.Other
+40-0Embeds the AWS audit-role CloudFormation template and adds Helm configuration for its public URL.
Written for commit 48afb16. Summary will update on new commits.