feat: netease_mail - #84
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
Summary by CodeRabbit
WalkthroughThis change generalizes the QQ Mail IMAP/SMTP implementation into shared mail actions, protocol types, error handling, and runtime execution. It adds configuration-driven credential hosts, limits, timeouts, and provider messages. QQ Mail now uses the shared runtime, while NetEase Mail receives actions, credential validation, provider metadata, and executors. The protocol supports shared send, search, fetch, attachment, folder, message-state, move, delete, reply, and forward operations. Sequence Diagram(s)sequenceDiagram
participant ProviderExecutor
participant executeMailAction
participant MailProtocol
participant IMAPSMTP
ProviderExecutor->>executeMailAction: dispatch mail action
executeMailAction->>MailProtocol: invoke mail operation
MailProtocol->>IMAPSMTP: connect and execute IMAP/SMTP request
IMAPSMTP-->>MailProtocol: return data or protocol error
MailProtocol-->>executeMailAction: return result or mapped error
executeMailAction-->>ProviderExecutor: return provider action response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mail/imap-smtp/protocol.ts (1)
175-198: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake the injected IMAP client contract match runtime usage.
createImapClientmay legally return a client implementing onlyconnect,logout,list, andclose, but action paths cast it toRuntimeImapClientand call methods such asmailboxOpen. A conforming injected client can therefore compile and crash at runtime.Proposed contract correction
export interface MailProtocolDependencies { createSmtpTransport?: (config: Record<string, unknown>) => MailSmtpTransport; - createImapClient?: (config: Record<string, unknown>) => MailImapClient; + createImapClient?: (config: Record<string, unknown>) => RuntimeImapClient; } -function createImapClient(...): MailImapClient { +function createImapClient(...): RuntimeImapClient { // ... } -return await callback(client as RuntimeImapClient); +return await callback(client);🤖 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 `@src/mail/imap-smtp/protocol.ts` around lines 175 - 198, Update the MailImapClient contract and related RuntimeImapClient usage so createImapClient requires every IMAP method invoked by the action paths, including mailboxOpen and any other runtime-called methods. Remove the mismatch between the injectable client type and RuntimeImapClient casts while preserving the existing connect, logout, close, and list members.
🧹 Nitpick comments (3)
src/providers/netease_mail/definition.ts (1)
26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the help URL into a provider-local constant.
The help-center URL is embedded inline in the field description.
♻️ Proposed refactor
+const authorizationCodeHelpUrl = + "https://help.mail.163.com/faqDetail.do?code=d7a5dc8471cd0c0e8b4b8f4f8e49998b374173cfe9171305fa1ce630d7f67ac2a5feb28b66796d3b"; + export const provider: ProviderDefinition = { ... { key: "authorizationCode", label: "Authorization Code", inputType: "password", required: true, secret: true, placeholder: "16-character code", description: - "The 16-character client authorization code created after enabling IMAP/SMTP in NetEase Mail settings: https://help.mail.163.com/faqDetail.do?code=d7a5dc8471cd0c0e8b4b8f4f8e49998b374173cfe9171305fa1ce630d7f67ac2a5feb28b66796d3b. This is not the NetEase Mail web login password.", + `The 16-character client authorization code created after enabling IMAP/SMTP in NetEase Mail settings: ${authorizationCodeHelpUrl}. This is not the NetEase Mail web login password.`, },As per path instructions for
src/providers/**/definition.ts: "Prefer provider-local constants for official scopes, permissions, URLs, and API versions."🤖 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 `@src/providers/netease_mail/definition.ts` around lines 26 - 34, Extract the embedded NetEase Mail help-center URL from the authorizationCode field description into a provider-local constant in the definition module, then interpolate or reference that constant in the description while preserving the existing text and URL.Source: Path instructions
src/mail/imap-smtp/protocol.ts (1)
144-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the public fetch option contracts.
fetchSummariesandfetchMessageexpose inline object types across a module boundary. Define reusable interfaces such asMailFetchSummariesOptionsandMailFetchMessageOptions.As per coding guidelines, “Prefer named options/input interfaces over inline object types when a function signature spans multiple lines or crosses module boundaries.”
🤖 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 `@src/mail/imap-smtp/protocol.ts` around lines 144 - 160, Define exported named interfaces for the fetch option contracts, such as MailFetchSummariesOptions and MailFetchMessageOptions, and replace the inline option object types in fetchSummaries and fetchMessage with those interfaces. Preserve the existing required properties and literal constraints.Source: Coding guidelines
src/mail/imap-smtp/runtime.ts (1)
54-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the handler map instead of repeating pass-through wrappers.
Every handler only forwards its own key to
executeMailAction. Export a shared action-name tuple and derive this map from it, keeping the action union, definitions, and executors synchronized without twelve wrappers.As per coding guidelines, “Avoid trivial pass-through helpers” and “Avoid repeated action-name wiring; define action handlers once and derive executor maps through shared provider runtime helpers.”
🤖 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 `@src/mail/imap-smtp/runtime.ts` around lines 54 - 93, Replace the repetitive wrappers in createMailActionHandlers with a derived handler map built from a shared exported mail action-name tuple. Use that tuple as the source for MailActionName and handler construction so action definitions and executors remain synchronized, while preserving each handler’s input/context forwarding to executeMailAction.Source: Coding guidelines
🤖 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 `@src/mail/imap-smtp/actions.ts`:
- Line 140: Replace the private mail.* aliases in each affected action’s
requiredScopes, including the entries near the listed symbols, with the
corresponding provider-native capabilities. Parameterize the action factory with
each provider’s native capabilities and align mailScopes accordingly; omit
requiredScopes for custom credentials that do not expose native scopes.
In `@src/mail/imap-smtp/protocol.ts`:
- Around line 442-446: Update downloadAttachment() to detect when
client.download(..., { maxBytes: mailAttachmentDownloadByteLimit }) reaches the
byte cap and fail instead of writing or returning a truncated attachment. Ensure
oversized downloads are rejected before exposing the file as successful, while
preserving the existing isFolderMissingError handling and expectedSize behavior
for complete downloads.
- Around line 90-101: Extend the MailSummary contract to expose normalized
Reply-To addresses from IMAP ENVELOPE data, preserving the field during message
fetching. Update reply_email recipient inference to prefer the Reply-To
addresses when present and fall back to from otherwise.
In `@src/mail/imap-smtp/temp-files.ts`:
- Around line 1-10: Update sanitizeTempFileName to cap the final sanitized
filename at a safe component length, while preserving a short extension when
present. Apply the limit after sanitization and edge trimming, retain the
existing "file" fallback, and ensure truncation does not leave unsafe trailing
separators or discard the preserved extension.
In `@src/providers/netease_mail/config.ts`:
- Around line 27-34: Update the domain lookup in the configuration flow using
serversByDomain so it accepts only own properties, such as via Object.hasOwn,
before reading the server entry. Preserve the existing ProviderRequestError 400
response for unsupported domains, including inherited keys like constructor and
__proto__.
---
Outside diff comments:
In `@src/mail/imap-smtp/protocol.ts`:
- Around line 175-198: Update the MailImapClient contract and related
RuntimeImapClient usage so createImapClient requires every IMAP method invoked
by the action paths, including mailboxOpen and any other runtime-called methods.
Remove the mismatch between the injectable client type and RuntimeImapClient
casts while preserving the existing connect, logout, close, and list members.
---
Nitpick comments:
In `@src/mail/imap-smtp/protocol.ts`:
- Around line 144-160: Define exported named interfaces for the fetch option
contracts, such as MailFetchSummariesOptions and MailFetchMessageOptions, and
replace the inline option object types in fetchSummaries and fetchMessage with
those interfaces. Preserve the existing required properties and literal
constraints.
In `@src/mail/imap-smtp/runtime.ts`:
- Around line 54-93: Replace the repetitive wrappers in createMailActionHandlers
with a derived handler map built from a shared exported mail action-name tuple.
Use that tuple as the source for MailActionName and handler construction so
action definitions and executors remain synchronized, while preserving each
handler’s input/context forwarding to executeMailAction.
In `@src/providers/netease_mail/definition.ts`:
- Around line 26-34: Extract the embedded NetEase Mail help-center URL from the
authorizationCode field description into a provider-local constant in the
definition module, then interpolate or reference that constant in the
description while preserving the existing text and URL.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e398587c-dbd3-4100-b941-187a84a2232b
📒 Files selected for processing (15)
src/mail/imap-smtp/actions.tssrc/mail/imap-smtp/config.tssrc/mail/imap-smtp/errors.tssrc/mail/imap-smtp/protocol.tssrc/mail/imap-smtp/runtime.test.tssrc/mail/imap-smtp/runtime.tssrc/mail/imap-smtp/temp-files.tssrc/providers/netease_mail/actions.tssrc/providers/netease_mail/config.tssrc/providers/netease_mail/definition.tssrc/providers/netease_mail/executors.tssrc/providers/qq_mail/actions.tssrc/providers/qq_mail/config.tssrc/providers/qq_mail/errors.tssrc/providers/qq_mail/executors.ts
💤 Files with no reviewable changes (1)
- src/providers/qq_mail/errors.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mail/imap-smtp/protocol.ts (1)
175-198: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake the injected IMAP client contract match runtime usage.
createImapClientmay legally return a client implementing onlyconnect,logout,list, andclose, but action paths cast it toRuntimeImapClientand call methods such asmailboxOpen. A conforming injected client can therefore compile and crash at runtime.Proposed contract correction
export interface MailProtocolDependencies { createSmtpTransport?: (config: Record<string, unknown>) => MailSmtpTransport; - createImapClient?: (config: Record<string, unknown>) => MailImapClient; + createImapClient?: (config: Record<string, unknown>) => RuntimeImapClient; } -function createImapClient(...): MailImapClient { +function createImapClient(...): RuntimeImapClient { // ... } -return await callback(client as RuntimeImapClient); +return await callback(client);🤖 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 `@src/mail/imap-smtp/protocol.ts` around lines 175 - 198, Update the MailImapClient contract and related RuntimeImapClient usage so createImapClient requires every IMAP method invoked by the action paths, including mailboxOpen and any other runtime-called methods. Remove the mismatch between the injectable client type and RuntimeImapClient casts while preserving the existing connect, logout, close, and list members.
🧹 Nitpick comments (3)
src/providers/netease_mail/definition.ts (1)
26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the help URL into a provider-local constant.
The help-center URL is embedded inline in the field description.
♻️ Proposed refactor
+const authorizationCodeHelpUrl = + "https://help.mail.163.com/faqDetail.do?code=d7a5dc8471cd0c0e8b4b8f4f8e49998b374173cfe9171305fa1ce630d7f67ac2a5feb28b66796d3b"; + export const provider: ProviderDefinition = { ... { key: "authorizationCode", label: "Authorization Code", inputType: "password", required: true, secret: true, placeholder: "16-character code", description: - "The 16-character client authorization code created after enabling IMAP/SMTP in NetEase Mail settings: https://help.mail.163.com/faqDetail.do?code=d7a5dc8471cd0c0e8b4b8f4f8e49998b374173cfe9171305fa1ce630d7f67ac2a5feb28b66796d3b. This is not the NetEase Mail web login password.", + `The 16-character client authorization code created after enabling IMAP/SMTP in NetEase Mail settings: ${authorizationCodeHelpUrl}. This is not the NetEase Mail web login password.`, },As per path instructions for
src/providers/**/definition.ts: "Prefer provider-local constants for official scopes, permissions, URLs, and API versions."🤖 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 `@src/providers/netease_mail/definition.ts` around lines 26 - 34, Extract the embedded NetEase Mail help-center URL from the authorizationCode field description into a provider-local constant in the definition module, then interpolate or reference that constant in the description while preserving the existing text and URL.Source: Path instructions
src/mail/imap-smtp/protocol.ts (1)
144-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the public fetch option contracts.
fetchSummariesandfetchMessageexpose inline object types across a module boundary. Define reusable interfaces such asMailFetchSummariesOptionsandMailFetchMessageOptions.As per coding guidelines, “Prefer named options/input interfaces over inline object types when a function signature spans multiple lines or crosses module boundaries.”
🤖 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 `@src/mail/imap-smtp/protocol.ts` around lines 144 - 160, Define exported named interfaces for the fetch option contracts, such as MailFetchSummariesOptions and MailFetchMessageOptions, and replace the inline option object types in fetchSummaries and fetchMessage with those interfaces. Preserve the existing required properties and literal constraints.Source: Coding guidelines
src/mail/imap-smtp/runtime.ts (1)
54-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the handler map instead of repeating pass-through wrappers.
Every handler only forwards its own key to
executeMailAction. Export a shared action-name tuple and derive this map from it, keeping the action union, definitions, and executors synchronized without twelve wrappers.As per coding guidelines, “Avoid trivial pass-through helpers” and “Avoid repeated action-name wiring; define action handlers once and derive executor maps through shared provider runtime helpers.”
🤖 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 `@src/mail/imap-smtp/runtime.ts` around lines 54 - 93, Replace the repetitive wrappers in createMailActionHandlers with a derived handler map built from a shared exported mail action-name tuple. Use that tuple as the source for MailActionName and handler construction so action definitions and executors remain synchronized, while preserving each handler’s input/context forwarding to executeMailAction.Source: Coding guidelines
🤖 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 `@src/mail/imap-smtp/actions.ts`:
- Line 140: Replace the private mail.* aliases in each affected action’s
requiredScopes, including the entries near the listed symbols, with the
corresponding provider-native capabilities. Parameterize the action factory with
each provider’s native capabilities and align mailScopes accordingly; omit
requiredScopes for custom credentials that do not expose native scopes.
In `@src/mail/imap-smtp/protocol.ts`:
- Around line 442-446: Update downloadAttachment() to detect when
client.download(..., { maxBytes: mailAttachmentDownloadByteLimit }) reaches the
byte cap and fail instead of writing or returning a truncated attachment. Ensure
oversized downloads are rejected before exposing the file as successful, while
preserving the existing isFolderMissingError handling and expectedSize behavior
for complete downloads.
- Around line 90-101: Extend the MailSummary contract to expose normalized
Reply-To addresses from IMAP ENVELOPE data, preserving the field during message
fetching. Update reply_email recipient inference to prefer the Reply-To
addresses when present and fall back to from otherwise.
In `@src/mail/imap-smtp/temp-files.ts`:
- Around line 1-10: Update sanitizeTempFileName to cap the final sanitized
filename at a safe component length, while preserving a short extension when
present. Apply the limit after sanitization and edge trimming, retain the
existing "file" fallback, and ensure truncation does not leave unsafe trailing
separators or discard the preserved extension.
In `@src/providers/netease_mail/config.ts`:
- Around line 27-34: Update the domain lookup in the configuration flow using
serversByDomain so it accepts only own properties, such as via Object.hasOwn,
before reading the server entry. Preserve the existing ProviderRequestError 400
response for unsupported domains, including inherited keys like constructor and
__proto__.
---
Outside diff comments:
In `@src/mail/imap-smtp/protocol.ts`:
- Around line 175-198: Update the MailImapClient contract and related
RuntimeImapClient usage so createImapClient requires every IMAP method invoked
by the action paths, including mailboxOpen and any other runtime-called methods.
Remove the mismatch between the injectable client type and RuntimeImapClient
casts while preserving the existing connect, logout, close, and list members.
---
Nitpick comments:
In `@src/mail/imap-smtp/protocol.ts`:
- Around line 144-160: Define exported named interfaces for the fetch option
contracts, such as MailFetchSummariesOptions and MailFetchMessageOptions, and
replace the inline option object types in fetchSummaries and fetchMessage with
those interfaces. Preserve the existing required properties and literal
constraints.
In `@src/mail/imap-smtp/runtime.ts`:
- Around line 54-93: Replace the repetitive wrappers in createMailActionHandlers
with a derived handler map built from a shared exported mail action-name tuple.
Use that tuple as the source for MailActionName and handler construction so
action definitions and executors remain synchronized, while preserving each
handler’s input/context forwarding to executeMailAction.
In `@src/providers/netease_mail/definition.ts`:
- Around line 26-34: Extract the embedded NetEase Mail help-center URL from the
authorizationCode field description into a provider-local constant in the
definition module, then interpolate or reference that constant in the
description while preserving the existing text and URL.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e398587c-dbd3-4100-b941-187a84a2232b
📒 Files selected for processing (15)
src/mail/imap-smtp/actions.tssrc/mail/imap-smtp/config.tssrc/mail/imap-smtp/errors.tssrc/mail/imap-smtp/protocol.tssrc/mail/imap-smtp/runtime.test.tssrc/mail/imap-smtp/runtime.tssrc/mail/imap-smtp/temp-files.tssrc/providers/netease_mail/actions.tssrc/providers/netease_mail/config.tssrc/providers/netease_mail/definition.tssrc/providers/netease_mail/executors.tssrc/providers/qq_mail/actions.tssrc/providers/qq_mail/config.tssrc/providers/qq_mail/errors.tssrc/providers/qq_mail/executors.ts
💤 Files with no reviewable changes (1)
- src/providers/qq_mail/errors.ts
🛑 Comments failed to post (5)
src/mail/imap-smtp/actions.ts (1)
140-140: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Replace private
mail.*aliases with provider-native capabilities.The runtime also reports these aliases as granted scopes, exposing an internal abstraction as provider capability data. Parameterize the factory with each provider’s native capabilities and align
mailScopes, or omitrequiredScopeswhere custom credentials do not expose native scopes.As per coding guidelines, “Action
requiredScopesshould use provider-native scopes/capabilities, not private internal aliases.”Also applies to: 150-150, 158-158, 187-187, 217-217, 239-239, 258-258, 277-277, 298-298, 317-317, 365-365, 373-373
🤖 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 `@src/mail/imap-smtp/actions.ts` at line 140, Replace the private mail.* aliases in each affected action’s requiredScopes, including the entries near the listed symbols, with the corresponding provider-native capabilities. Parameterize the action factory with each provider’s native capabilities and align mailScopes accordingly; omit requiredScopes for custom credentials that do not expose native scopes.Source: Coding guidelines
src/mail/imap-smtp/protocol.ts (2)
90-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve
Reply-Towhen fetching messages.The fetched-message contract only retains
from, soreply_emailsends to the author even when the message specifies a different Reply-To address. IMAP ENVELOPE explicitly includes a reply-to field. (datatracker.ietf.org)Expose normalized Reply-To addresses and prefer them over
fromwhen inferring recipients.#!/bin/bash set -euo pipefail # Confirm the repository's declared ImapFlow version. fd -H -t f '^(package\.json|pnpm-lock\.yaml|package-lock\.json|yarn\.lock|bun\.lock)$' -0 | xargs -0 rg -n -C2 '\bimapflow\b' # Inspect the exact package contract used by this PR. tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT tarball="$(curl -fsSL https://registry.npmjs.org/imapflow/1.4.6 | jq -r '.dist.tarball')" curl -fsSL "$tarball" -o "$tmp/imapflow.tgz" tar -xzf "$tmp/imapflow.tgz" -C "$tmp" rg -n -C4 '\breplyTo\b|reply-to' "$tmp/package/lib"Also applies to: 129-136
🤖 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 `@src/mail/imap-smtp/protocol.ts` around lines 90 - 101, Extend the MailSummary contract to expose normalized Reply-To addresses from IMAP ENVELOPE data, preserving the field during message fetching. Update reply_email recipient inference to prefer the Reply-To addresses when present and fall back to from otherwise.
442-446: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail fd -H -t f '^(package\.json|pnpm-lock\.yaml|package-lock\.json|yarn\.lock|bun\.lock)$' -0 | xargs -0 rg -n -C2 '\bimapflow\b' tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT tarball="$(curl -fsSL https://registry.npmjs.org/imapflow/1.4.6 | jq -r '.dist.tarball')" curl -fsSL "$tarball" -o "$tmp/imapflow.tgz" tar -xzf "$tmp/imapflow.tgz" -C "$tmp" rg -n -C8 '\bmaxBytes\b|LimitedPassThrough' "$tmp/package/lib"Repository: oomol-lab/open-connector
Length of output: 14573
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant implementation and nearby attachment handling. file="src/mail/imap-smtp/protocol.ts" wc -l "$file" sed -n '380,520p' "$file" # Find any handling of attachment size, truncation, or expectedSize in this area. rg -n -C3 'expectedSize|maxBytes|attachment|truncate|truncat|download' src/mail/imap-smtp/protocol.ts src/mail -g '!**/*.map'Repository: oomol-lab/open-connector
Length of output: 50380
Reject oversized attachments instead of returning a partial file.
client.download(..., { maxBytes })can stop at the byte cap, butdownloadAttachment()still writes that stream to disk and returnsexpectedSizefrom metadata. Fail the download when the cap is hit so a too-large attachment is never exposed as a successful, truncated file.🤖 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 `@src/mail/imap-smtp/protocol.ts` around lines 442 - 446, Update downloadAttachment() to detect when client.download(..., { maxBytes: mailAttachmentDownloadByteLimit }) reaches the byte cap and fail instead of writing or returning a truncated attachment. Ensure oversized downloads are rejected before exposing the file as successful, while preserving the existing isFolderMissingError handling and expectedSize behavior for complete downloads.src/mail/imap-smtp/temp-files.ts (1)
1-10: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Bound the sanitized filename length.
Input filenames have no schema length limit, so a long valid name can still make temporary-file creation fail with
ENAMETOOLONG. Cap the ASCII result to a safe component length while preserving a short extension.🤖 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 `@src/mail/imap-smtp/temp-files.ts` around lines 1 - 10, Update sanitizeTempFileName to cap the final sanitized filename at a safe component length, while preserving a short extension when present. Apply the limit after sanitization and edge trimming, retain the existing "file" fallback, and ensure truncation does not leave unsafe trailing separators or discard the preserved extension.src/providers/netease_mail/config.ts (1)
27-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
node -e 'const o = {"163.com": 1}; console.log(!!o["__proto__"], !!o["constructor"]);'Repository: oomol-lab/open-connector
Length of output: 172
Guard the domain lookup with an own-property check
serversByDomain[domain]can still resolve inherited keys likeconstructoror__proto__, which bypasses the unsupported-domain check and returns an incomplete credential instead of a 400. UseObject.hasOwn(or aMap) here.Suggested fix
const domain = parts[1].toLowerCase(); - const servers = serversByDomain[domain as keyof typeof serversByDomain]; + const servers = Object.hasOwn(serversByDomain, domain) + ? serversByDomain[domain as keyof typeof serversByDomain] + : undefined; if (!servers) {📝 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 domain = parts[1].toLowerCase(); const servers = Object.hasOwn(serversByDomain, domain) ? serversByDomain[domain as keyof typeof serversByDomain] : undefined; if (!servers) { throw new ProviderRequestError( 400, "NetEase Mail supports only 163.com, 126.com, and yeah.net personal accounts.", ); }🤖 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 `@src/providers/netease_mail/config.ts` around lines 27 - 34, Update the domain lookup in the configuration flow using serversByDomain so it accepts only own properties, such as via Object.hasOwn, before reading the server entry. Preserve the existing ProviderRequestError 400 response for unsupported domains, including inherited keys like constructor and __proto__.
1 participant
No description provided.