features: added google sheet update, clear, append ops. google oauth …#81
Conversation
…is splited into gsheet_oauth and gmail_oauth and UI/UX fixed for node config
📝 WalkthroughWalkthroughGoogle OAuth now supports type-specific credentials and encoded redirect state. Google Sheets adds append, write, clear, header, and mapping flows across the node runtime, backend routes, and workflow UI. Node execution propagates authentication type and returns transaction results after status persistence. ChangesGoogle OAuth and credential lifecycle
Google Sheets operations
Execution routing
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/http-backend/src/routes/google_callback.tsOops! Something went wrong! :( ESLint: 9.32.0 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './library.js' is not defined by "exports" in /node_modules/@workspace/eslint-config/package.json apps/http-backend/src/routes/sheet.routes.tsOops! Something went wrong! :( ESLint: 9.32.0 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './library.js' is not defined by "exports" in /node_modules/@workspace/eslint-config/package.json apps/http-backend/src/routes/userRoutes/executionRoutes.tsOops! Something went wrong! :( ESLint: 9.32.0 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './library.js' is not defined by "exports" in /node_modules/@workspace/eslint-config/package.json
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
apps/web/app/workflows/[id]/components/ConfigModal.tsx (2)
677-698: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUnreachable duplicate render block. The new
isRequiredreturn at Lines 656-676 makes this secondreturndead code — delete it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 677 - 698, Remove the unreachable duplicate JSX return block containing the input field after the new isRequired return; keep the earlier isRequired rendering path and surrounding component logic unchanged.
450-464: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
cred.config.emailwill throw ifconfigis absent, and the chip below still reads the oldcred.email. The dropdown crashes the whole modal render for any credential row without aconfigpayload, and Line 462 now always falls back tocred.name.🐛 Proposed fix
- {cred.config.email || cred.name || "Google Account"} + {cred.config?.email || cred.name || "Google Account"}- {cred.email || cred.name} + {cred.config?.email || cred.name}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 450 - 464, Update both credential display expressions in the credentials map blocks to safely handle missing config and use the same email source: read the email from cred.config only when config exists, then fall back to cred.name and the existing Google Account default for the dropdown; update the chip to use the same safe email-or-name value instead of cred.email.packages/nodes/src/google-sheets/google-sheets.node.ts (1)
10-32: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign the node schema with the Google Sheets runtime contract.
GoogleSheetNode.definition.config.fieldsis persisted as the node config, but the executor readsspreadsheetId,sheetName, and operation-specific flags; this schema currently requiresrangefor every operation and doesn’t include the fields that actually drive execution. Add/keep the necessary fields in the definition, or avoid persisting it as the source of truth so required-field checks don’t block valid workflows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/google-sheets/google-sheets.node.ts` around lines 10 - 32, Align GoogleSheetNode.definition.config.fields with the executor’s expected config by using spreadsheetId, sheetName, and the operation-specific flags instead of requiring range universally. Ensure required-field validation permits valid operations that do not use range, and update the schema symbols in GoogleSheetNode so persisted configuration matches runtime property names.packages/nodes/src/common/google-oauth-service.ts (2)
55-75: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winLogging the full
userinfo.data(includes the user's email) to console.
console.log("User Info: ", userinfo.data);writes PII (the authenticated Google account's email) to server logs. Prefer logging only non-identifying fields, or drop this debug log.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/common/google-oauth-service.ts` around lines 55 - 75, Remove the console.log call in getTokens that outputs userinfo.data, or replace it with logging only non-identifying fields; do not log the authenticated user's email or other PII.
77-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winLogging the full created credential record, including access/refresh tokens, to console.
console.log("THis log is writing ... ", Data)prints the entire Prismacredential.createresult, whoseconfigfield is the rawOAuthTokens(access_token, refresh_token). Logging OAuth secrets is a real exposure risk in any shared/aggregated logging system.🛡️ Proposed fix
- console.log("THis log is writing to see if google auth tokens is storing to db or not ", Data) + console.log("Credential saved:", { id: Data.id, userId: Data.userId, type: Data.type });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/common/google-oauth-service.ts` around lines 77 - 101, Remove the debug console.log in saveCredentials that prints the full Data credential record, including OAuth access and refresh tokens. Keep credential creation and existing error handling unchanged, and do not log the sensitive config or created record.apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
190-221: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
data: credentialsreturns raw OAuth tokens (access_token/refresh_token) to the client.
authService.getAllCredentials(userId, type)returns the full Prismacredentialrecords, whoseconfigfield is the rawOAuthTokensobject (access_token, refresh_token, scope, etc.). This route serializes that array directly into the response body, so the frontend (and browser devtools/network logs) receives live OAuth secrets. Stripconfig(or map to only{ id, type, createdAt }) before returning.🛡️ Proposed fix
const credentials = await authService.getAllCredentials(userId, type) + const sanitizedCredentials = credentials.map(({ config, ...rest }) => rest); ... return res.status(statusCodes.OK).json({ message: "Credentials fetched", - data: credentials, - hasCredentials: credentials.length > 0, + data: sanitizedCredentials, + hasCredentials: credentials.length > 0, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts` around lines 190 - 221, Update the response in the credentials route after authService.getAllCredentials(userId, type) so it maps records to safe metadata only, such as id, type, and createdAt, excluding the config field and all OAuth tokens before assigning the result to data. Preserve hasCredentials based on whether credentials were found.apps/http-backend/src/routes/userRoutes/executionRoutes.ts (1)
40-100: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftExternal API call (
ExecutionRegister.execute) runs inside the Prisma$transaction, with no timeout.Gmail/Sheets executors can perform Google API calls and token refreshes; running that inside
$transactionholds the underlying DB connection/transaction open for the duration of that network I/O. Under latency or rate-limiting from Google, this risks Prisma's transaction timeout, connection-pool exhaustion, and blocking other DB work on the same pool. Consider creating the initial records, runningExecutionRegister.executeoutside any transaction, then persisting the final status in a short separate transaction (or non-transactional updates).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/http-backend/src/routes/userRoutes/executionRoutes.ts` around lines 40 - 100, Move ExecutionRegister.execute outside the prismaClient.$transaction callback so external API calls do not hold a database transaction open. Create the initial workflowExecution and nodeExecution records first, execute the operation, then persist the completed or failed statuses and outputs in a short separate transaction or standalone updates while preserving the existing success and failure results.
🧹 Nitpick comments (8)
packages/nodes/src/google-sheets/google-sheets.service.ts (2)
12-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType naming/duplication nit.
ReadRowsParams_ClearRowsreads awkwardly andAppendRowsParams/WriteRowsParamsare structurally identical. ConsiderRangeParams+ a singleValuesParams extends RangeParams { values: any[][] }.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/google-sheets/google-sheets.service.ts` around lines 12 - 28, Rename ReadRowsParams_ClearRows to RangeParams, introduce a shared ValuesParams extending RangeParams with the values field, and replace the structurally identical AppendRowsParams and WriteRowsParams usages with ValuesParams throughout the service.
59-77: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
if (files)is always truthy — the failure branch is unreachable.drive.files.listeither resolves with a response object or throws. Consider checkingfiles.data?.filesand letting/handling thrown errors explicitly, consistent with the newtry/catchstyle used elsewhere in this class.♻️ Suggested shape
async getSheets(): Promise<any> { - const files = await this.drive.files.list({ - q: "mimeType='application/vnd.google-apps.spreadsheet'", - spaces: 'drive', - pageSize: 10, - fields: 'files(id, name, createdTime)', - }) - - if (files) { - return { - success: true, - data: files - } - } - return { - success: false, - data: null - } + try { + const files = await this.drive.files.list({ + q: "mimeType='application/vnd.google-apps.spreadsheet'", + spaces: 'drive', + pageSize: 10, + fields: 'files(id, name, createdTime)', + }) + return { success: true, data: files } + } catch (error) { + return { + success: false, + data: null, + error: error instanceof Error ? error.message : 'Failed to list spreadsheets' + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/google-sheets/google-sheets.service.ts` around lines 59 - 77, Update getSheets to check whether the response contains files via files.data?.files rather than testing the always-truthy response object. Preserve the success shape for valid results, and add explicit try/catch handling so drive.files.list errors return the established failure response or are propagated consistently with the class.apps/http-backend/src/routes/sheet.routes.ts (1)
130-133: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDon't echo the raw executor result to the client.
resultmay carryauthUrland internal error text; returnresult.erroronly.🔒 Proposed fix
if (!result.success) return res.status(statusCodes.NOT_FOUND).json({ - message: "Failed to fetch headers", error: result + message: "Failed to fetch headers", + error: result.error ?? "Unknown error" });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/http-backend/src/routes/sheet.routes.ts` around lines 130 - 133, Update the !result.success response in the sheet route to return only result.error instead of serializing the entire executor result, while preserving the existing NOT_FOUND status and message.apps/web/app/workflows/[id]/components/ConfigModal.tsx (1)
398-424: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isFieldVisibletakesformDatabut Lines 419 and 422 read theconfigclosure instead. They happen to be the same object today; make it consistent so the helper stays pure.♻️ Proposed fix
if (field.name === 'mappedColumns') - return (config.mappingMode ?? 'visual') === 'visual' + return (formData.mappingMode ?? 'visual') === 'visual' if (field.name === 'bulkValues') - return config.mappingMode === 'bulk' + return formData.mappingMode === 'bulk'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 398 - 424, Update isFieldVisible to use its formData parameter for the mappedColumns and bulkValues visibility checks instead of reading the config closure, preserving the existing mappingMode defaults and bulk-mode behavior while keeping the helper pure.packages/nodes/src/google-sheets/google-sheets.executor.ts (2)
374-384: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidate
rangebefore building values.prepareValuesForSheetperforms a network header fetch (and can throw) before the cheap!rangeguard runs, so a misconfigured node surfaces a header-fetch error instead of the intended message. Move the guard above Line 377.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` around lines 374 - 384, Move the !range validation in the Update Rows execution flow before calling prepareValuesForSheet(context), while retaining the existing required-range error response. Keep spreadsheetId extraction and subsequent value preparation unchanged for valid ranges.
211-241: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
prepareValuesForSheetre-resolves credentials on every call.getHeaderRow→ensureCredentials→getCredentialshits the DB again inside an already-authenticated execute path. Consider extracting a privatefetchHeaders(sheetService, ...)that reusesthis.sheetService.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` around lines 211 - 241, The mapping branch of prepareValuesForSheet should fetch headers through a private fetchHeaders method that accepts and reuses this.sheetService, rather than calling getHeaderRow and triggering ensureCredentials/getCredentials again. Preserve the existing header-error handling and mapped-row fallback behavior.apps/web/app/lib/types/node.types.ts (1)
49-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
bulk_payloadhas no renderer and no config uses it.ConfigModal.renderFieldwould fall through to<input type="bulk_payload">. Drop it until it's implemented, or add the branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/lib/types/node.types.ts` around lines 49 - 57, Remove "bulk_payload" from the field type union in the node field type definition, since ConfigModal.renderField has no corresponding renderer and no configuration currently uses it. Leave the existing supported field types unchanged.apps/http-backend/src/routes/google_callback.ts (1)
39-55: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo whitelist validation of OAuth
type/authTypeacross the flow. The value only gets a truthy-check at intake, then flows unvalidated through scope selection and into persisted credential rows, risking typo'd/unrecognized types that no downstreamgetAllCredentials/getCredentialsfilter will ever match.
apps/http-backend/src/routes/google_callback.ts#L39-L55: validateauthTypeagainst a fixed set (e.g.['gmail_oauth', 'gsheet_oauth']) instead of only checking truthiness.packages/nodes/src/common/google-oauth-service.ts#L28-L53: replace thetype.includes('mail')substring check ingetAuthUrlwith an exact match against a typed union (type: 'gmail_oauth' | 'gsheet_oauth') or a scope lookup map.packages/nodes/src/common/google-oauth-service.ts#L77-L95: constrainsaveCredentials'stypeparameter to the same union so only recognized credential types are ever persisted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/http-backend/src/routes/google_callback.ts` around lines 39 - 55, Validate authType in the google callback route against the fixed gmail_oauth and gsheet_oauth set, rejecting unrecognized values before creating the authorization URL. In GoogleOAuthService.getAuthUrl, replace substring-based scope selection with exact typed-union matching or a scope lookup map. Constrain GoogleOAuthService.saveCredentials to the same typed union so only recognized credential types can be persisted; apply these changes at apps/http-backend/src/routes/google_callback.ts lines 39-55, packages/nodes/src/common/google-oauth-service.ts lines 28-53, and packages/nodes/src/common/google-oauth-service.ts lines 77-95.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/http-backend/src/routes/google_callback.ts`:
- Around line 116-200: Protect both the /debug/config and /debug/credentials
handlers in googleAuth with the existing userMiddleware, matching the
authenticated route pattern used elsewhere in the file and userRoutes.ts.
Additionally restrict /debug/credentials to an approved admin or
non-production-only access path, ensuring unauthenticated callers cannot
retrieve credential metadata for any user.
- Around line 74-101: Validate the decoded state.userId against the
authenticated user from req.user before calling authService.saveCredentials in
the OAuth callback. Reject the callback when the IDs are missing or do not
match, and only persist tokens for the authenticated session; do not use the
unsigned state.userId alone for authorization.
- Around line 69-80: Move the state decoding and JSON parsing into the existing
try block around the Google OAuth callback flow so missing or malformed state
follows the intended error redirect path. Keep redirect_uri scoped for both
success and catch handling, and remove the catch block’s redundant state
re-decode by reusing the parsed value.
- Around line 36-55: Update the userId assignment in the Google OAuth initiation
flow to read the authenticated subject from req.user.sub instead of req.user.id.
Keep the existing force-unwrapped value passed to GoogleOAuthService.getAuthUrl,
ensuring the authenticated user identifier is encoded into the OAuth state.
In `@apps/http-backend/src/routes/userRoutes/executionRoutes.ts`:
- Around line 91-98: Update the workflowExecution failure update in the
execution route to persist executionResult.error rather than
executionResult.output in its error field. Keep the existing failed status and
completion timestamp behavior unchanged, matching the nodeExecution failure
branch.
- Around line 102-112: The failed branch in the node execution response,
identified by the result.success check, incorrectly returns
statusCodes.FORBIDDEN for non-authorization failures. Replace it with an
appropriate client-error status such as 400 or 422, or retain the existing
success/accepted status while communicating failure through
result.executionResult; leave the successful branch unchanged.
In `@apps/web/app/hooks/useCredential.ts`:
- Around line 20-30: Remove the full response serialization and credential
payload logging in the useCredential request flow, retaining only non-sensitive
diagnostics if needed. In the response.hasCredentials branch, clear the existing
authUrl state before setting the stored credential so switching from an
unconnected integration cannot retain a stale Connect URL.
In `@apps/web/app/lib/api.ts`:
- Around line 149-157: Update getHeaders in apps/web/app/lib/api.ts (lines
149-157) to send sheetName as a query parameter instead of interpolating it into
the URL path. Update the /getHeaders route in
apps/http-backend/src/routes/sheet.routes.ts (lines 106-128) to remove
:sheetName, read req.query.sheetName, and preserve the existing presence
validation.
In `@apps/web/app/lib/nodeConfigs/gmail.action.ts`:
- Line 9: Update the Gmail and Google Sheets credential configurations in
gmail.action.ts and googleSheet.action.ts to support the new gmail_oauth and
gsheet_oauth types, and add a Prisma SQL/seed backfill mapping existing
google_oauth credentials to the appropriate provider type. Update
GoogleOAuthService and legacy credential paths to exclude or migrate hidden
google_oauth records so credential selection and execution no longer require
re-authentication.
In `@apps/web/app/lib/nodeConfigs/googleSheet.action.ts`:
- Line 55: Remove the stray leading double quote from the user-facing
description value in the Google Sheets action configuration, leaving the
intended description text unchanged.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 52-53: Use loadedSheetRef in the header-fetch effect around
getHeaders to identify the current sheet/request and ignore stale responses from
earlier sheetName or operation changes. Update the ref when starting the active
fetch, and only apply results to sheetHeaders when the returned request still
matches the current ref; ensure rapid changes cannot leave headers from a
previous selection.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts`:
- Around line 92-99: Use the same safe auth-type fallback when calling
oauthService.getAuthUrl in the authorization-required paths at the locations
around lines 112 and 171: pass context.authType || 'gsheet_oauth' instead of the
potentially undefined context.authType, matching the existing fallback used when
retrieving credentials.
- Around line 335-345: The string-handling branch in the rawValues normalization
flow should attempt strict JSON.parse on the trimmed input before applying the
single-quote conversion heuristic. Update the parsing logic around
normalizeValues so valid JSON containing apostrophes is returned normally, and
use the quote-swapped parse only for non-strict input; preserve the existing
[[trimmed]] fallback when both parses fail.
- Around line 444-452: Update executeClearRows so that a configured custom range
is prefixed with context.config.sheetName before being passed to
sheetService.clearRows. Preserve the existing clearEntireTable and
includeHeaderRow range behavior, and ensure all ranges target the intended sheet
tab.
- Line 94: Remove the credential-object logging around getCredentials and the
getAllCredentials result in the Google Sheets executor. Do not print tokens or
credential/config objects; preserve the existing credential retrieval and
execution behavior.
---
Outside diff comments:
In `@apps/http-backend/src/routes/userRoutes/executionRoutes.ts`:
- Around line 40-100: Move ExecutionRegister.execute outside the
prismaClient.$transaction callback so external API calls do not hold a database
transaction open. Create the initial workflowExecution and nodeExecution records
first, execute the operation, then persist the completed or failed statuses and
outputs in a short separate transaction or standalone updates while preserving
the existing success and failure results.
In `@apps/http-backend/src/routes/userRoutes/userRoutes.ts`:
- Around line 190-221: Update the response in the credentials route after
authService.getAllCredentials(userId, type) so it maps records to safe metadata
only, such as id, type, and createdAt, excluding the config field and all OAuth
tokens before assigning the result to data. Preserve hasCredentials based on
whether credentials were found.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 677-698: Remove the unreachable duplicate JSX return block
containing the input field after the new isRequired return; keep the earlier
isRequired rendering path and surrounding component logic unchanged.
- Around line 450-464: Update both credential display expressions in the
credentials map blocks to safely handle missing config and use the same email
source: read the email from cred.config only when config exists, then fall back
to cred.name and the existing Google Account default for the dropdown; update
the chip to use the same safe email-or-name value instead of cred.email.
In `@packages/nodes/src/common/google-oauth-service.ts`:
- Around line 55-75: Remove the console.log call in getTokens that outputs
userinfo.data, or replace it with logging only non-identifying fields; do not
log the authenticated user's email or other PII.
- Around line 77-101: Remove the debug console.log in saveCredentials that
prints the full Data credential record, including OAuth access and refresh
tokens. Keep credential creation and existing error handling unchanged, and do
not log the sensitive config or created record.
In `@packages/nodes/src/google-sheets/google-sheets.node.ts`:
- Around line 10-32: Align GoogleSheetNode.definition.config.fields with the
executor’s expected config by using spreadsheetId, sheetName, and the
operation-specific flags instead of requiring range universally. Ensure
required-field validation permits valid operations that do not use range, and
update the schema symbols in GoogleSheetNode so persisted configuration matches
runtime property names.
---
Nitpick comments:
In `@apps/http-backend/src/routes/google_callback.ts`:
- Around line 39-55: Validate authType in the google callback route against the
fixed gmail_oauth and gsheet_oauth set, rejecting unrecognized values before
creating the authorization URL. In GoogleOAuthService.getAuthUrl, replace
substring-based scope selection with exact typed-union matching or a scope
lookup map. Constrain GoogleOAuthService.saveCredentials to the same typed union
so only recognized credential types can be persisted; apply these changes at
apps/http-backend/src/routes/google_callback.ts lines 39-55,
packages/nodes/src/common/google-oauth-service.ts lines 28-53, and
packages/nodes/src/common/google-oauth-service.ts lines 77-95.
In `@apps/http-backend/src/routes/sheet.routes.ts`:
- Around line 130-133: Update the !result.success response in the sheet route to
return only result.error instead of serializing the entire executor result,
while preserving the existing NOT_FOUND status and message.
In `@apps/web/app/lib/types/node.types.ts`:
- Around line 49-57: Remove "bulk_payload" from the field type union in the node
field type definition, since ConfigModal.renderField has no corresponding
renderer and no configuration currently uses it. Leave the existing supported
field types unchanged.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 398-424: Update isFieldVisible to use its formData parameter for
the mappedColumns and bulkValues visibility checks instead of reading the config
closure, preserving the existing mappingMode defaults and bulk-mode behavior
while keeping the helper pure.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts`:
- Around line 374-384: Move the !range validation in the Update Rows execution
flow before calling prepareValuesForSheet(context), while retaining the existing
required-range error response. Keep spreadsheetId extraction and subsequent
value preparation unchanged for valid ranges.
- Around line 211-241: The mapping branch of prepareValuesForSheet should fetch
headers through a private fetchHeaders method that accepts and reuses
this.sheetService, rather than calling getHeaderRow and triggering
ensureCredentials/getCredentials again. Preserve the existing header-error
handling and mapped-row fallback behavior.
In `@packages/nodes/src/google-sheets/google-sheets.service.ts`:
- Around line 12-28: Rename ReadRowsParams_ClearRows to RangeParams, introduce a
shared ValuesParams extending RangeParams with the values field, and replace the
structurally identical AppendRowsParams and WriteRowsParams usages with
ValuesParams throughout the service.
- Around line 59-77: Update getSheets to check whether the response contains
files via files.data?.files rather than testing the always-truthy response
object. Preserve the success shape for valid results, and add explicit try/catch
handling so drive.files.list errors return the established failure response or
are propagated consistently with the class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 850e9eb4-633d-49d8-8a09-8080f6ecf0d0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.gitignoreapps/http-backend/src/routes/google_callback.tsapps/http-backend/src/routes/sheet.routes.tsapps/http-backend/src/routes/userRoutes/executionRoutes.tsapps/http-backend/src/routes/userRoutes/userRoutes.tsapps/http-backend/src/services/token-refresh.service.tsapps/web/app/hooks/useCredential.tsapps/web/app/lib/api.tsapps/web/app/lib/nodeConfigs/gmail.action.tsapps/web/app/lib/nodeConfigs/googleSheet.action.tsapps/web/app/lib/types/node.types.tsapps/web/app/workflows/[id]/components/ConfigModal.tsxapps/web/package.jsonpackages/nodes/src/common/google-oauth-service.tspackages/nodes/src/gmail/gmail.executor.tspackages/nodes/src/gmail/gmail.node.tspackages/nodes/src/google-sheets/google-sheets.executor.tspackages/nodes/src/google-sheets/google-sheets.node.tspackages/nodes/src/google-sheets/google-sheets.service.tspackages/ui/src/styles/globals.csspnpm-workspace.yaml
| // const userId = req.user?.id || "test_user"; // Get from auth middleware | ||
| const userId = req.user?.id; | ||
| const workflowId = req.query.workflowId as string | undefined; | ||
| const authType = req.query.authType as string; | ||
|
|
||
| // Ensure redirect URI matches Google Cloud Console configuration | ||
| const redirectUri = REDIRECT_URI; | ||
|
|
||
| const redirectUri = (req.query.redirect_url as string) || (workflowId ? `/workflows/${workflowId}` : '/workflows'); | ||
| console.log(` redirect from google calback before auth: ${redirectUri}`) | ||
| if (!authType) return res.status(statusCodes.BAD_REQUEST).json({ | ||
| error: "Auth Type is required", | ||
| success: false, | ||
| }) | ||
|
|
||
| console.log("🔐 OAuth Initiate - Redirect URI:", redirectUri); | ||
| console.log("🔐 OAuth Initiate - User ID:", userId); | ||
| console.log("🔐 OAuth Initiate - Workflow ID:", workflowId || "NOT PROVIDED"); | ||
|
|
||
| // Encode userId and workflowId in state (format: userId|workflowId) | ||
| const state = workflowId ? `${userId}|${workflowId}` : userId; | ||
|
|
||
| const oauth2 = new google.auth.OAuth2( | ||
| process.env.GOOGLE_CLIENT_ID, | ||
| process.env.GOOGLE_CLIENT_SECRET, | ||
| redirectUri | ||
| ); | ||
|
|
||
| const authUrl = oauth2.generateAuthUrl({ | ||
| access_type: "offline", | ||
| scope: [ | ||
| "https://www.googleapis.com/auth/spreadsheets", | ||
| "https://www.googleapis.com/auth/drive.readonly", | ||
| "https://www.googleapis.com/auth/gmail.send", | ||
| "https://www.googleapis.com/auth/gmail.readonly", | ||
| ], | ||
| state: state, | ||
| prompt: "consent", | ||
| }); | ||
| const authService = new GoogleOAuthService(); | ||
| const authUrl = authService.getAuthUrl(userId!, authType, redirectUri) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether req.user carries `.id` or only `.sub`
rg -n --type=ts -C2 'req\.user(\?)?\.(id|sub)\b' apps/http-backend/src
rg -n --type=ts 'interface JwtPayload' -C5Repository: Dev-Pross/BuildFlow
Length of output: 8964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -i 'google_callback|auth|middleware|jwt|prisma|google_oauth|oauth' apps/http-backend/src -t f
echo
echo "== google_callback outline =="
ast-grep outline apps/http-backend/src/routes/google_callback.ts || true
echo
echo "== google_callback relevant lines =="
sed -n '1,180p' apps/http-backend/src/routes/google_callback.ts
echo
echo "== auth request/middlewares definitions =="
rg -n --type=ts -C3 'AuthRequest|userMiddleware|verify|jwt|jwtDecode|JWT' apps/http-backend/src | head -n 240Repository: Dev-Pross/BuildFlow
Length of output: 25815
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== userMiddleware.ts full =="
cat -n apps/http-backend/src/routes/userRoutes/userMiddleware.ts
echo
echo "== package token/auth deps (focused) =="
rg 'next-auth|jsonwebtoken|jsonwebtoken|`@types/jsonwebtoken`' apps/http-backend -g 'package.json' -g 'pnpm-lock.yaml' -g 'yarn.lock' -g 'package-lock.json' || true
echo
echo "== GoogleOAuthService occurrences =="
fd -p 'google|oauth|auth' src -t f -i | sed 's#^`#apps/http-backend/src/`#' || true
rg -n --type=ts 'class GoogleOAuthService|saveCredentials|getAuthUrl|generateAuthUrl|secret' apps -C3 || trueRepository: Dev-Pross/BuildFlow
Length of output: 4555
🌐 Web query:
next-auth JWT token default sub field jwtPayload id
💡 Result:
In NextAuth.js (Auth.js), the sub (subject) field in the JWT payload is the standard, built-in field used to store the unique user identifier [1][2]. Key details regarding the sub field and JWT payload: 1. Default Behavior: The sub field is part of the DefaultJWT interface [1][3]. When a user signs in, NextAuth.js automatically maps the user's unique ID to the sub claim in the JWT [4][5]. 2. Persistence: Because sub is included by default, it is available in the jwt callback [6]. You can access it via token.sub [5]. 3. Customization: While sub is the standard location for the user ID, it is not uncommon for developers to explicitly add the user's ID to a different custom property (e.g., token.id or token.uid) within the jwt callback to simplify access or maintain specific naming conventions [6][5]. 4. Accessing the ID: - To use the default: Access token.sub [5]. - To add a custom ID field: javascript async jwt({ token, user }) { if (user) { token.id = user.id; // Adds 'id' to the JWT payload } return token; } 5. Client-side Exposure: By default, the sub field is often omitted from the session object returned to the client for security reasons [6]. To make the ID (whether stored in sub or a custom field) available to the client, you must explicitly forward it in the session callback [6][5]: javascript async session({ session, token }) { if (session.user) { session.user.id = token.sub; // Or token.id } return session; } In summary, sub is the intended field for the user ID in the JWT payload [2][5], but you have full flexibility to store it elsewhere or map it to other properties via the jwt and session callbacks [6].
Citations:
- 1: https://github.com/nextauthjs/next-auth/blob/b4ef14ab/packages/core/src/jwt.ts
- 2: https://deepwiki.com/nextauthjs/next-auth/2.4-jwt-and-tokens
- 3: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/jwt.ts
- 4: JWT sub property nextauthjs/next-auth#6631
- 5: https://logfetch.com/next-auth-get-user-database-id-from-session/
- 6: https://next-auth.js.org/configuration/callbacks
Use the authenticated user id from req.user.sub.
This middleware assigns the NextAuth JWT payload to req.user, whose subject claim is sub; id is not set by the existing code. Since userId is force-unwrapped and encoded into the OAuth state, a missing sub leaves the callback without the user id to persist credentials.
🐛 Proposed fix
- const userId = req.user?.id;
+ const userId = req.user?.sub;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // const userId = req.user?.id || "test_user"; // Get from auth middleware | |
| const userId = req.user?.id; | |
| const workflowId = req.query.workflowId as string | undefined; | |
| const authType = req.query.authType as string; | |
| // Ensure redirect URI matches Google Cloud Console configuration | |
| const redirectUri = REDIRECT_URI; | |
| const redirectUri = (req.query.redirect_url as string) || (workflowId ? `/workflows/${workflowId}` : '/workflows'); | |
| console.log(` redirect from google calback before auth: ${redirectUri}`) | |
| if (!authType) return res.status(statusCodes.BAD_REQUEST).json({ | |
| error: "Auth Type is required", | |
| success: false, | |
| }) | |
| console.log("🔐 OAuth Initiate - Redirect URI:", redirectUri); | |
| console.log("🔐 OAuth Initiate - User ID:", userId); | |
| console.log("🔐 OAuth Initiate - Workflow ID:", workflowId || "NOT PROVIDED"); | |
| // Encode userId and workflowId in state (format: userId|workflowId) | |
| const state = workflowId ? `${userId}|${workflowId}` : userId; | |
| const oauth2 = new google.auth.OAuth2( | |
| process.env.GOOGLE_CLIENT_ID, | |
| process.env.GOOGLE_CLIENT_SECRET, | |
| redirectUri | |
| ); | |
| const authUrl = oauth2.generateAuthUrl({ | |
| access_type: "offline", | |
| scope: [ | |
| "https://www.googleapis.com/auth/spreadsheets", | |
| "https://www.googleapis.com/auth/drive.readonly", | |
| "https://www.googleapis.com/auth/gmail.send", | |
| "https://www.googleapis.com/auth/gmail.readonly", | |
| ], | |
| state: state, | |
| prompt: "consent", | |
| }); | |
| const authService = new GoogleOAuthService(); | |
| const authUrl = authService.getAuthUrl(userId!, authType, redirectUri) | |
| // const userId = req.user?.id || "test_user"; // Get from auth middleware | |
| const userId = req.user?.sub; | |
| const workflowId = req.query.workflowId as string | undefined; | |
| const authType = req.query.authType as string; | |
| // Ensure redirect URI matches Google Cloud Console configuration | |
| const redirectUri = (req.query.redirect_url as string) || (workflowId ? `/workflows/${workflowId}` : '/workflows'); | |
| console.log(` redirect from google calback before auth: ${redirectUri}`) | |
| if (!authType) return res.status(statusCodes.BAD_REQUEST).json({ | |
| error: "Auth Type is required", | |
| success: false, | |
| }) | |
| console.log("🔐 OAuth Initiate - Redirect URI:", redirectUri); | |
| console.log("🔐 OAuth Initiate - User ID:", userId); | |
| console.log("🔐 OAuth Initiate - Workflow ID:", workflowId || "NOT PROVIDED"); | |
| const authService = new GoogleOAuthService(); | |
| const authUrl = authService.getAuthUrl(userId!, authType, redirectUri) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/http-backend/src/routes/google_callback.ts` around lines 36 - 55, Update
the userId assignment in the Google OAuth initiation flow to read the
authenticated subject from req.user.sub instead of req.user.id. Keep the
existing force-unwrapped value passed to GoogleOAuthService.getAuthUrl, ensuring
the authenticated user identifier is encoded into the OAuth state.
| if (!code || typeof code !== "string") { | ||
| return res.json({ error: "Missing or invalid authorization code" }); | ||
| } | ||
|
|
||
| // Ensure redirect URI matches Google Cloud Console configuration | ||
| const redirectUri = REDIRECT_URI; | ||
|
|
||
| // Parse state: format is "userId" or "userId|workflowId" | ||
| let userId: string | undefined; | ||
| let workflowId: string | undefined; | ||
|
|
||
| if (state && typeof state === "string") { | ||
| const parts = state.split("|"); | ||
| userId = parts[0]; | ||
| workflowId = parts[1]; // Will be undefined if not provided | ||
| } | ||
|
|
||
| console.log("🔐 OAuth Callback - Redirect URI:", redirectUri); | ||
| const authService = new GoogleOAuthService() | ||
|
|
||
| const { userId, type, redirect_uri } = JSON.parse(Buffer.from(state as string, 'base64').toString()) | ||
| console.log("🔐 OAuth Callback - Redirect URI:", FRONTEND_URL + redirect_uri); | ||
| console.log("🔐 OAuth Callback - Received code:", code ? "✅ Present" : "❌ Missing"); | ||
| console.log("🔐 OAuth Callback - State:", state); | ||
| console.log("🔐 OAuth Callback - Parsed User ID:", userId); | ||
| console.log("🔐 OAuth Callback - Parsed Workflow ID:", workflowId || "NOT PROVIDED"); | ||
|
|
||
| const oauth2 = new google.auth.OAuth2( | ||
| process.env.GOOGLE_CLIENT_ID, | ||
| process.env.GOOGLE_CLIENT_SECRET, | ||
| redirectUri | ||
| ); | ||
|
|
||
| try { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
state is decoded outside the try block — malformed/missing state crashes unhandled.
code is validated before use, but state is not: if state is absent or not valid base64/JSON, Buffer.from/JSON.parse on Line 75 throws before the surrounding try (Line 80) can catch it, bypassing the intended graceful redirect-on-error flow.
🛡️ Proposed fix
+ if (!state || typeof state !== "string") {
+ return res.status(statusCodes.BAD_REQUEST).json({ error: "Missing or invalid state" });
+ }
+
const authService = new GoogleOAuthService()
-
- const { userId, type, redirect_uri } = JSON.parse(Buffer.from(state as string, 'base64').toString())
+ let userId: string, type: string, redirect_uri: string;
+ try {
+ ({ userId, type, redirect_uri } = JSON.parse(Buffer.from(state, 'base64').toString()));
+ } catch {
+ return res.status(statusCodes.BAD_REQUEST).json({ error: "Invalid state payload" });
+ }
console.log("🔐 OAuth Callback - Redirect URI:", FRONTEND_URL + redirect_uri);This also removes the need for the redundant re-decode in the catch block below (Line 110), since redirect_uri would already be in scope.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!code || typeof code !== "string") { | |
| return res.json({ error: "Missing or invalid authorization code" }); | |
| } | |
| // Ensure redirect URI matches Google Cloud Console configuration | |
| const redirectUri = REDIRECT_URI; | |
| // Parse state: format is "userId" or "userId|workflowId" | |
| let userId: string | undefined; | |
| let workflowId: string | undefined; | |
| if (state && typeof state === "string") { | |
| const parts = state.split("|"); | |
| userId = parts[0]; | |
| workflowId = parts[1]; // Will be undefined if not provided | |
| } | |
| console.log("🔐 OAuth Callback - Redirect URI:", redirectUri); | |
| const authService = new GoogleOAuthService() | |
| const { userId, type, redirect_uri } = JSON.parse(Buffer.from(state as string, 'base64').toString()) | |
| console.log("🔐 OAuth Callback - Redirect URI:", FRONTEND_URL + redirect_uri); | |
| console.log("🔐 OAuth Callback - Received code:", code ? "✅ Present" : "❌ Missing"); | |
| console.log("🔐 OAuth Callback - State:", state); | |
| console.log("🔐 OAuth Callback - Parsed User ID:", userId); | |
| console.log("🔐 OAuth Callback - Parsed Workflow ID:", workflowId || "NOT PROVIDED"); | |
| const oauth2 = new google.auth.OAuth2( | |
| process.env.GOOGLE_CLIENT_ID, | |
| process.env.GOOGLE_CLIENT_SECRET, | |
| redirectUri | |
| ); | |
| try { | |
| if (!code || typeof code !== "string") { | |
| return res.json({ error: "Missing or invalid authorization code" }); | |
| } | |
| if (!state || typeof state !== "string") { | |
| return res.status(statusCodes.BAD_REQUEST).json({ error: "Missing or invalid state" }); | |
| } | |
| const authService = new GoogleOAuthService() | |
| let userId: string, type: string, redirect_uri: string; | |
| try { | |
| ({ userId, type, redirect_uri } = JSON.parse(Buffer.from(state, 'base64').toString())); | |
| } catch { | |
| return res.status(statusCodes.BAD_REQUEST).json({ error: "Invalid state payload" }); | |
| } | |
| console.log("🔐 OAuth Callback - Redirect URI:", FRONTEND_URL + redirect_uri); | |
| console.log("🔐 OAuth Callback - Received code:", code ? "✅ Present" : "❌ Missing"); | |
| try { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/http-backend/src/routes/google_callback.ts` around lines 69 - 80, Move
the state decoding and JSON parsing into the existing try block around the
Google OAuth callback flow so missing or malformed state follows the intended
error redirect path. Keep redirect_uri scoped for both success and catch
handling, and remove the catch block’s redundant state re-decode by reusing the
parsed value.
|
|
||
| const { userId, type, redirect_uri } = JSON.parse(Buffer.from(state as string, 'base64').toString()) | ||
| console.log("🔐 OAuth Callback - Redirect URI:", FRONTEND_URL + redirect_uri); | ||
| console.log("🔐 OAuth Callback - Received code:", code ? "✅ Present" : "❌ Missing"); | ||
| console.log("🔐 OAuth Callback - State:", state); | ||
| console.log("🔐 OAuth Callback - Parsed User ID:", userId); | ||
| console.log("🔐 OAuth Callback - Parsed Workflow ID:", workflowId || "NOT PROVIDED"); | ||
|
|
||
| const oauth2 = new google.auth.OAuth2( | ||
| process.env.GOOGLE_CLIENT_ID, | ||
| process.env.GOOGLE_CLIENT_SECRET, | ||
| redirectUri | ||
| ); | ||
|
|
||
| try { | ||
| const { tokens } = await oauth2.getToken(code); | ||
|
|
||
| // DEBUG: Log tokens received from Google | ||
| console.log('\n🔐 Google OAuth Callback - Tokens received:'); | ||
| console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing'); | ||
| console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing'); | ||
| console.log(' expiry_date:', tokens.expiry_date); | ||
| console.log(' token_type:', tokens.token_type); | ||
| console.log(' scope:', tokens.scope); | ||
| if (!tokens.refresh_token) { | ||
| console.warn('⚠️ WARNING: No refresh_token received! User may have already authorized this app.'); | ||
| console.warn(' To force new refresh_token, user needs to revoke access at: https://myaccount.google.com/permissions'); | ||
| } | ||
|
|
||
| // Save tokens to database if userId is provided | ||
| if (userId) { | ||
| console.log(' Saving tokens for userId:', userId); | ||
| await Oauth.saveCredentials(userId, tokens as OAuthTokens) | ||
| console.log(' ✅ Tokens saved to database'); | ||
| } | ||
| const tokens = await authService.getTokens(code); | ||
|
|
||
| // DEBUG: Log tokens received from Google | ||
| console.log('\n🔐 Google OAuth Callback - Tokens received:'); | ||
| console.log(' access_token:', tokens.access_token ? '✅ Present' : '❌ Missing'); | ||
| console.log(' refresh_token:', tokens.refresh_token ? '✅ Present' : '❌ Missing'); | ||
| console.log(' expiry_date:', tokens.expiry_date); | ||
| console.log(' token_type:', tokens.token_type); | ||
| console.log(' scope:', tokens.scope); | ||
|
|
||
| if (!tokens.refresh_token) { | ||
| console.warn('⚠️ WARNING: No refresh_token received! User may have already authorized this app.'); | ||
| console.warn(' To force new refresh_token, user needs to revoke access at: https://myaccount.google.com/permissions'); | ||
| } | ||
|
|
||
| // Save tokens to database if userId is provided | ||
| if (userId) { | ||
| console.log(' Saving tokens for userId:', userId); | ||
| await authService.saveCredentials(userId, tokens as OAuthTokens, type) | ||
| console.log(' ✅ Tokens saved to database'); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
state.userId is never verified against the authenticated session — unsigned state used for an authorization decision.
The base64 state payload is not signed/HMAC'd, and req.user (populated by userMiddleware) is never cross-checked against state.userId. An authenticated attacker can complete their own Google consent flow, tamper with state to substitute a victim's userId, and hit this callback to have saveCredentials(victimUserId, attackerTokens, type) create a new credential row owned by the victim but containing the attacker's tokens. Depending on downstream credential-selection logic (getCredentials orders by id desc), this can redirect a victim's future Sheets/Gmail node executions to attacker-controlled resources.
Bind the decoded userId to the authenticated session before persisting credentials.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/http-backend/src/routes/google_callback.ts` around lines 74 - 101,
Validate the decoded state.userId against the authenticated user from req.user
before calling authService.saveCredentials in the OAuth callback. Reject the
callback when the IDs are missing or do not match, and only persist tokens for
the authenticated session; do not use the unsigned state.userId alone for
authorization.
| googleAuth.get('/debug/config', async (req: Request, res: Response) => { | ||
| try { | ||
| const redirectUri = REDIRECT_URI; | ||
| const clientId = process.env.GOOGLE_CLIENT_ID; | ||
| const hasClientSecret = !!process.env.GOOGLE_CLIENT_SECRET; | ||
|
|
||
| // Create a test OAuth2 client to verify configuration | ||
| const oauth2 = new google.auth.OAuth2( | ||
| clientId, | ||
| process.env.GOOGLE_CLIENT_SECRET, | ||
| redirectUri | ||
| ); | ||
|
|
||
| const testAuthUrl = oauth2.generateAuthUrl({ | ||
| access_type: "offline", | ||
| scope: ["https://www.googleapis.com/auth/spreadsheets"], | ||
| state: "test", | ||
| prompt: "consent", | ||
| }); | ||
|
|
||
| const urlObj = new URL(testAuthUrl); | ||
| const redirectUriInUrl = urlObj.searchParams.get('redirect_uri'); | ||
|
|
||
| return res.json({ | ||
| environment: { | ||
| GOOGLE_REDIRECT_URI: redirectUri, | ||
| GOOGLE_CLIENT_ID: clientId ? `${clientId.substring(0, 20)}...` : "NOT SET", | ||
| GOOGLE_CLIENT_SECRET: hasClientSecret ? "SET" : "NOT SET", | ||
| }, | ||
| oauth2Client: { | ||
| redirectUri: redirectUri, | ||
| redirectUriInGeneratedUrl: redirectUriInUrl, | ||
| matches: redirectUri === redirectUriInUrl, | ||
| }, | ||
| testAuthUrl: testAuthUrl, | ||
| message: redirectUri === redirectUriInUrl | ||
| ? "✅ Configuration looks correct!" | ||
| : "❌ Redirect URI mismatch detected!" | ||
| }); | ||
| } catch (err) { | ||
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ | ||
| error: err instanceof Error ? err.message : 'Unknown error', | ||
| stack: err instanceof Error ? err.stack : undefined | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| // Debug endpoint to check stored credentials | ||
| googleAuth.get('/debug/credentials', async(req: Request, res: Response)=>{ | ||
| try { | ||
| const { prismaClient } = await import('@repo/db/client'); | ||
| const credentials = await prismaClient.credential.findMany({ | ||
| where: { type: 'google_oauth' }, | ||
| select: { | ||
| id: true, | ||
| userId: true, | ||
| type: true, | ||
| config: true | ||
| } | ||
| }); | ||
|
|
||
| const debugInfo = credentials.map(cred => { | ||
| const config = cred.config as any; | ||
| return { | ||
| id: cred.id, | ||
| userId: cred.userId, | ||
| hasAccessToken: !!config?.access_token, | ||
| hasRefreshToken: !!config?.refresh_token, | ||
| refreshTokenLength: config?.refresh_token?.length || 0, | ||
| expiryDate: config?.expiry_date, | ||
| expiresIn: config?.expiry_date ? Math.round((config.expiry_date - Date.now()) / 1000 / 60) + ' minutes' : 'N/A', | ||
| isInvalid: config?.invalid || false, | ||
| scope: config?.scope | ||
| }; | ||
| }); | ||
|
|
||
| console.log('\n📋 Stored Credentials Debug:'); | ||
| console.table(debugInfo); | ||
|
|
||
| return res.json({ credentials: debugInfo }); | ||
| } catch (err) { | ||
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ error: err instanceof Error ? err.message : 'Unknown error' }); | ||
| } | ||
| googleAuth.get('/debug/credentials', async (req: Request, res: Response) => { | ||
| try { | ||
| const { prismaClient } = await import('@repo/db/client'); | ||
|
|
||
| const credentials = await prismaClient.credential.findMany({ | ||
| where: { type: { in: ['gmail_oauth', 'gsheet_oauth', 'google_oauth'] } }, | ||
| select: { | ||
| id: true, | ||
| userId: true, | ||
| type: true, | ||
| config: true | ||
| } | ||
| }); | ||
|
|
||
| const debugInfo = credentials.map(cred => { | ||
| const config = cred.config as any; | ||
| return { | ||
| id: cred.id, | ||
| userId: cred.userId, | ||
| hasAccessToken: !!config?.access_token, | ||
| hasRefreshToken: !!config?.refresh_token, | ||
| refreshTokenLength: config?.refresh_token?.length || 0, | ||
| expiryDate: config?.expiry_date, | ||
| expiresIn: config?.expiry_date ? Math.round((config.expiry_date - Date.now()) / 1000 / 60) + ' minutes' : 'N/A', | ||
| isInvalid: config?.invalid || false, | ||
| scope: config?.scope | ||
| }; | ||
| }); | ||
|
|
||
| console.log('\n📋 Stored Credentials Debug:'); | ||
| console.table(debugInfo); | ||
|
|
||
| return res.json({ credentials: debugInfo }); | ||
| } catch (err) { | ||
| return res.status(statusCodes.INTERNAL_SERVER_ERROR).json({ error: err instanceof Error ? err.message : 'Unknown error' }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Debug endpoints have no auth — /debug/credentials publicly leaks credential metadata (userId, token presence/expiry/scope) for every user, now across a widened type set.
Neither /debug/config nor /debug/credentials apply userMiddleware (contrast with every other route in this file and userRoutes.ts). /debug/credentials is the more serious exposure: it returns userId, hasAccessToken, hasRefreshToken, refreshTokenLength, expiryDate, and scope for all gmail_oauth/gsheet_oauth/google_oauth credentials in the database to any unauthenticated caller. This should require auth (and likely be gated to admins/non-production only).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/http-backend/src/routes/google_callback.ts` around lines 116 - 200,
Protect both the /debug/config and /debug/credentials handlers in googleAuth
with the existing userMiddleware, matching the authenticated route pattern used
elsewhere in the file and userRoutes.ts. Additionally restrict
/debug/credentials to an approved admin or non-production-only access path,
ensuring unauthenticated callers cannot retrieve credential metadata for any
user.
| await tx.workflowExecution.update({ | ||
| where: {id: workflowExecution.id}, | ||
| data:{ | ||
| where: { id: workflowExecution.id }, | ||
| data: { | ||
| status: "Failed", | ||
| completedAt: new Date(), | ||
| error: executionResult.output | ||
| } | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
workflowExecution.error uses executionResult.output instead of executionResult.error on failure.
The nodeExecution failure branch correctly persists error: executionResult.error (Line 88-89), but the workflowExecution failure branch persists error: executionResult.output (Line 96-97). On failure, executors typically don't populate output (see GmailExecutor.execute's catch path, which only returns error), so workflowExecution.error will end up undefined/empty, losing the actual failure reason for the workflow-level record.
🐛 Proposed fix
await tx.workflowExecution.update({
where: { id: workflowExecution.id },
data: {
status: "Failed",
completedAt: new Date(),
- error: executionResult.output
+ error: executionResult.error
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await tx.workflowExecution.update({ | |
| where: {id: workflowExecution.id}, | |
| data:{ | |
| where: { id: workflowExecution.id }, | |
| data: { | |
| status: "Failed", | |
| completedAt: new Date(), | |
| error: executionResult.output | |
| } | |
| }) | |
| await tx.workflowExecution.update({ | |
| where: { id: workflowExecution.id }, | |
| data: { | |
| status: "Failed", | |
| completedAt: new Date(), | |
| error: executionResult.error | |
| } | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/http-backend/src/routes/userRoutes/executionRoutes.ts` around lines 91 -
98, Update the workflowExecution failure update in the execution route to
persist executionResult.error rather than executionResult.output in its error
field. Keep the existing failed status and completion timestamp behavior
unchanged, matching the nodeExecution failure branch.
| // Add a ref to track which sheet's headers are currently loaded | ||
| const loadedSheetRef = useRef<string>(""); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
loadedSheetRef is declared but never used, and the header fetch it was meant to guard is unprotected. Concurrent/rapid sheetName/operation changes issue overlapping getHeaders calls whose responses can land out of order, leaving sheetHeaders from a previously selected sheet. Wire the ref (or a request token) into the fetch.
♻️ Sketch
if (credentialId && spreadsheetId && sheetName && ["append_rows", "write_rows"].includes(operation)) {
+ const key = `${credentialId}:${spreadsheetId}:${sheetName}`;
+ loadedSheetRef.current = key;
setIsLoadingHeaders(true);
try {
const headers = await api.google.getHeaders(credentialId, spreadsheetId, sheetName);
- setSheetHeaders(headers || []);
+ if (loadedSheetRef.current === key) setSheetHeaders(headers || []);
} catch (e) {
console.error("Failed to load sheet headers", e);
} finally {
- setIsLoadingHeaders(false);
+ if (loadedSheetRef.current === key) setIsLoadingHeaders(false);
}
}Also applies to: 324-338
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 52 - 53,
Use loadedSheetRef in the header-fetch effect around getHeaders to identify the
current sheet/request and ignore stale responses from earlier sheetName or
operation changes. Update the ref when starting the active fetch, and only apply
results to sheetHeaders when the returned request still matches the current ref;
ensure rapid changes cannot leave headers from a previous selection.
| const type = context.authType ? context.authType : 'gsheet_oauth' | ||
| const credentials = await this.oauthService.getCredentials(context.userId, context.credentialId, type); | ||
| console.log("credentails from sheet.executor: ", credentials) | ||
| if (!credentials) { | ||
| return { | ||
| success: false, | ||
| error: 'Google Sheets authorization required', | ||
| authUrl: this.oauthService.getAuthUrl(context.userId), | ||
| authUrl: this.oauthService.getAuthUrl(context.userId, context.authType), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
getAuthUrl(context.userId, context.authType) can throw when authType is missing. Line 92 defensively defaults authType, which shows the field is not guaranteed at runtime despite the interface. GoogleOAuthService.getAuthUrl does type.includes('mail'), so an undefined authType turns the "authorization required" path into a TypeError instead of a helpful auth prompt. Same at Lines 112 and 171.
🐛 Proposed fix
- const type = context.authType ? context.authType : 'gsheet_oauth'
+ const type = context.authType || 'gsheet_oauth'
const credentials = await this.oauthService.getCredentials(context.userId, context.credentialId, type);
- console.log("credentails from sheet.executor: ", credentials)
if (!credentials) {
return {
success: false,
error: 'Google Sheets authorization required',
- authUrl: this.oauthService.getAuthUrl(context.userId, context.authType),
+ authUrl: this.oauthService.getAuthUrl(context.userId, type),Apply the same context.authType || 'gsheet_oauth' fallback at Lines 112 and 171.
Also applies to: 112-112, 171-171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` around lines 92 -
99, Use the same safe auth-type fallback when calling oauthService.getAuthUrl in
the authorization-required paths at the locations around lines 112 and 171: pass
context.authType || 'gsheet_oauth' instead of the potentially undefined
context.authType, matching the existing fallback used when retrieving
credentials.
| console.log("credentails from sheet.executor: ",credentials) | ||
| const type = context.authType ? context.authType : 'gsheet_oauth' | ||
| const credentials = await this.oauthService.getCredentials(context.userId, context.credentialId, type); | ||
| console.log("credentails from sheet.executor: ", credentials) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't log the credential object — it contains OAuth access/refresh tokens. getCredentials returns { id, tokens }; this writes bearer and refresh tokens to stdout/log aggregation. Same concern for Line 59 (getAllCredentials result includes config with tokens).
🔒 Proposed fix
- console.log("credentails from sheet.executor: ", credentials)
+ console.log("[GoogleSheets] credential resolved:", credentials?.id ?? 'none')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log("credentails from sheet.executor: ", credentials) | |
| console.log("[GoogleSheets] credential resolved:", credentials?.id ?? 'none') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` at line 94,
Remove the credential-object logging around getCredentials and the
getAllCredentials result in the Google Sheets executor. Do not print tokens or
credential/config objects; preserve the existing credential retrieval and
execution behavior.
| if (typeof rawValues === 'string') { | ||
| const trimmed = rawValues.trim(); | ||
| // Handle single quotes if user typed [['a', 'b']] | ||
| const formattedJson = trimmed.replace(/'/g, '"'); | ||
| try { | ||
| const parsed = JSON.parse(formattedJson); | ||
| return this.normalizeValues(parsed); | ||
| } catch { | ||
| return [[trimmed]]; | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Blanket ' → " replacement corrupts legitimate string data. For valid JSON input containing apostrophes ([["O'Brien"]]), the replacement produces invalid JSON, JSON.parse throws, and the fallback writes the entire payload into a single cell. Try strict JSON.parse first and only fall back to the quote-swap heuristic.
🐛 Proposed fix
if (typeof rawValues === 'string') {
const trimmed = rawValues.trim();
- // Handle single quotes if user typed [['a', 'b']]
- const formattedJson = trimmed.replace(/'/g, '"');
try {
- const parsed = JSON.parse(formattedJson);
- return this.normalizeValues(parsed);
+ return this.normalizeValues(JSON.parse(trimmed));
} catch {
- return [[trimmed]];
+ // Fallback: user may have typed [['a', 'b']] with single quotes
+ try {
+ return this.normalizeValues(JSON.parse(trimmed.replace(/'/g, '"')));
+ } catch {
+ return [[trimmed]];
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof rawValues === 'string') { | |
| const trimmed = rawValues.trim(); | |
| // Handle single quotes if user typed [['a', 'b']] | |
| const formattedJson = trimmed.replace(/'/g, '"'); | |
| try { | |
| const parsed = JSON.parse(formattedJson); | |
| return this.normalizeValues(parsed); | |
| } catch { | |
| return [[trimmed]]; | |
| } | |
| } | |
| if (typeof rawValues === 'string') { | |
| const trimmed = rawValues.trim(); | |
| try { | |
| return this.normalizeValues(JSON.parse(trimmed)); | |
| } catch { | |
| // Fallback: user may have typed [['a', 'b']] with single quotes | |
| try { | |
| return this.normalizeValues(JSON.parse(trimmed.replace(/'/g, '"'))); | |
| } catch { | |
| return [[trimmed]]; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` around lines 335
- 345, The string-handling branch in the rawValues normalization flow should
attempt strict JSON.parse on the trimmed input before applying the single-quote
conversion heuristic. Update the parsing logic around normalizeValues so valid
JSON containing apostrophes is returned normally, and use the quote-swapped
parse only for non-strict input; preserve the existing [[trimmed]] fallback when
both parses fail.
| async executeClearRows(sheetService: GoogleSheetsService, context: NodeExecutionContext): Promise<NodeExecutionResult> { | ||
| try { | ||
| const spreadsheetId = context.config.spreadsheetId; | ||
| const range = (!context.config.clearEntireTable && context.config.range) ? context.config.range : (context.config.includeHeaderRow ? `${context.config.sheetName}!A1:Z` : `${context.config.sheetName}!A2:Z`); | ||
|
|
||
| const response = await sheetService.clearRows({ | ||
| spreadsheetId: spreadsheetId, | ||
| range: range | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Clear with a custom range omits the sheet-name prefix, so the wrong tab gets cleared. When clearEntireTable is false, range is passed through as e.g. A5:C20; without ${sheetName}! the Sheets API resolves it against the first tab in the spreadsheet. The read and write paths both prefix the sheet name — this path must too. This is destructive data loss on the wrong sheet.
🐛 Proposed fix
- const range = (!context.config.clearEntireTable && context.config.range) ? context.config.range : (context.config.includeHeaderRow ? `${context.config.sheetName}!A1:Z` : `${context.config.sheetName}!A2:Z`);
+ const sheetName = context.config.sheetName;
+ const range = (!context.config.clearEntireTable && context.config.range)
+ ? `${sheetName}!${context.config.range}`
+ : (context.config.includeHeaderRow ? `${sheetName}!A1:Z` : `${sheetName}!A2:Z`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async executeClearRows(sheetService: GoogleSheetsService, context: NodeExecutionContext): Promise<NodeExecutionResult> { | |
| try { | |
| const spreadsheetId = context.config.spreadsheetId; | |
| const range = (!context.config.clearEntireTable && context.config.range) ? context.config.range : (context.config.includeHeaderRow ? `${context.config.sheetName}!A1:Z` : `${context.config.sheetName}!A2:Z`); | |
| const response = await sheetService.clearRows({ | |
| spreadsheetId: spreadsheetId, | |
| range: range | |
| }) | |
| async executeClearRows(sheetService: GoogleSheetsService, context: NodeExecutionContext): Promise<NodeExecutionResult> { | |
| try { | |
| const spreadsheetId = context.config.spreadsheetId; | |
| const sheetName = context.config.sheetName; | |
| const range = (!context.config.clearEntireTable && context.config.range) | |
| ? `${sheetName}!${context.config.range}` | |
| : (context.config.includeHeaderRow ? `${sheetName}!A1:Z` : `${sheetName}!A2:Z`); | |
| const response = await sheetService.clearRows({ | |
| spreadsheetId: spreadsheetId, | |
| range: range | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/google-sheets/google-sheets.executor.ts` around lines 444
- 452, Update executeClearRows so that a configured custom range is prefixed
with context.config.sheetName before being passed to sheetService.clearRows.
Preserve the existing clearEntireTable and includeHeaderRow range behavior, and
ensure all ranges target the intended sheet tab.
2 participants
…is splited into gsheet_oauth and gmail_oauth and UI/UX fixed for node config
Summary by CodeRabbit