Skip to content

Fix file uploads#1715

Merged
maor-rozenfeld merged 1 commit intoopenops-cloud:mainfrom
hyperglance:fix-file-property-schema-and-match-pattern
Dec 10, 2025
Merged

Fix file uploads#1715
maor-rozenfeld merged 1 commit intoopenops-cloud:mainfrom
hyperglance:fix-file-property-schema-and-match-pattern

Conversation

@rashmi-sy
Copy link
Copy Markdown
Contributor

@rashmi-sy rashmi-sy commented Dec 2, 2025

This PR fixes the error encountered when using file uploading actions: "Expected a file url or base64 with mimeType".
The error occurs even for valid file uploads due incorrect regex and schema checks. The issue was not detected due to a faulty unit test which is also addressed by this PR.

Additional Notes

  • Fixes regex rejecting valid mime types such as application/vnd.ms-visio.drawing
  • Fixes the Zod schema check for PropertyType.FILE to check for WorkflowFile not record<>
  • Fixes the unit-test that was incorrectly passing due to using the same displayName for both test cases which caused the output to contain only 1 error instead of 2.

Testing Checklist

Check all that apply:

  • I tested the feature thoroughly, including edge cases

  • I verified all affected areas still work as expected

  • Automated tests were added/updated if necessary

  • Changes are backwards compatible with any existing data, otherwise a migration script is provided

Fixes OPS-3230.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced file data URL processing to support MIME types with extended character formats.
    • Improved file property validation with stricter type checking for improved reliability.
  • Tests

    • Updated test naming for improved clarity in file validation scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 2, 2025

Walkthrough

Updates file validation logic by modifying the Base64 data URL regex to accept MIME types with dots, replacing generic object validation with strict WorkflowFile instance checking in props validation, and updating test property naming for consistency.

Changes

Cohort / File(s) Summary
File MIME type regex enhancement
packages/engine/src/lib/variables/processors/file.ts
Modified Base64 data URL regex pattern to allow dots (.) in MIME type group: /^data:([A-Za-z-+/]+);base64,(.+)$//^data:([A-Za-z.+/-]+);base64,(.+)$/. Broadens accepted MIME types without altering subsequent processing logic.
File property validation schema tightening
packages/engine/src/lib/variables/props-processor.ts
Replaced generic object-record schema with strict instance validation using z.instanceof(WorkflowFile, ...) for FILE property. Adds WorkflowFile import. Type validation enforcement tightened; error messages and control flow unchanged.
Test property naming update
packages/engine/test/services/props-processor.test.ts
Updated displayName for base64WithMime property from 'Base64' to 'Base64WithMime'. Validation logic and test processing remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • packages/engine/src/lib/variables/processors/file.ts: Regex pattern change requires careful validation to ensure the new character set ([A-Za-z.+/-]) correctly captures intended MIME types without introducing unintended matches.
  • packages/engine/src/lib/variables/props-processor.ts: Schema migration from generic to instance-checked validation warrants verification that WorkflowFile instances are properly constructed throughout the codebase.
  • packages/engine/test/services/props-processor.test.ts: Verify that the displayName change aligns with test expectations and downstream error reporting.

Poem

🐰 A dot in the MIME type appears,
WorkflowFile now stands crystal-clear—
No more generic, loose, and mild,
Just validated files, refined and styled!
Test names bloom like carrots true,
File handlers now validated through and through! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix file uploads' is vague and generic, failing to specify the actual technical fixes (regex pattern for MIME types and schema validation). Consider a more descriptive title such as 'Fix file property schema validation and MIME type regex pattern' to clearly communicate the specific changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is mostly complete with clear explanations of what was fixed, why it was changed, and testing checklist items marked as completed.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Dec 2, 2025

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Dec 2, 2025

Greptile Overview

Greptile Summary

Fixed file upload validation that was incorrectly rejecting valid MIME types and file instances. The PR addresses three interconnected issues:

  • Regex pattern fix: Updated MIME type regex in file.ts:34 to include dot character, now accepting vendor-specific MIME types like application/vnd.ms-visio.drawing
  • Schema validation fix: Changed Zod schema from z.record() to z.instanceof(WorkflowFile) in props-processor.ts:196 for proper type checking
  • Test fix: Corrected test to use unique displayName values (Base64WithMime vs Base64), ensuring both test cases are properly validated instead of one being silently dropped

The changes are minimal, focused, and directly address the root causes identified in the issue.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects surgical fixes to critical bugs with clear test coverage. Changes are minimal, focused, and directly address root causes without introducing additional complexity or side effects.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
packages/engine/src/lib/variables/processors/file.ts 5/5 Fixed regex pattern to accept dot character in MIME types (e.g., application/vnd.ms-visio.drawing)
packages/engine/src/lib/variables/props-processor.ts 5/5 Fixed Zod schema validation to check for WorkflowFile instance instead of generic record
packages/engine/test/services/props-processor.test.ts 5/5 Fixed test to use unique displayName values, ensuring proper validation of both test cases

Sequence Diagram

sequenceDiagram
    participant User
    participant PropsProcessor
    participant FileProcessor
    participant Validator
    
    User->>PropsProcessor: Upload file (base64 or URL)
    PropsProcessor->>FileProcessor: Process file input
    
    alt Base64 with MIME type
        FileProcessor->>FileProcessor: Check isBase64()
        FileProcessor->>FileProcessor: Match regex pattern
        Note over FileProcessor: Fixed regex now includes dot<br/>to support MIME types like<br/>application/vnd.ms-visio.drawing
        FileProcessor->>FileProcessor: Extract MIME type and base64 data
        FileProcessor->>FileProcessor: Create WorkflowFile instance
        FileProcessor-->>PropsProcessor: Return WorkflowFile
    else URL
        FileProcessor->>FileProcessor: Download from URL
        FileProcessor->>FileProcessor: Create WorkflowFile instance
        FileProcessor-->>PropsProcessor: Return WorkflowFile
    end
    
    PropsProcessor->>Validator: Validate processed value
    Validator->>Validator: Check instanceof WorkflowFile
    Note over Validator: Fixed validation changed from<br/>z.record() to z.instanceof(WorkflowFile)<br/>for proper type checking
    
    alt Valid WorkflowFile
        Validator-->>PropsProcessor: Validation passed
        PropsProcessor-->>User: Success
    else Invalid type
        Validator-->>PropsProcessor: Error message
        PropsProcessor-->>User: Error: Expected WorkflowFile
    end
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/engine/src/lib/variables/processors/file.ts (1)

31-40: Regex fix looks good; consider supporting digits in MIME types

Allowing . in the MIME portion correctly enables data URLs like application/vnd.ms-visio.drawing while keeping the regex anchored and safe.

Since you’re already touching this, consider also allowing digits in the MIME token to cover common types with numeric suffixes (e.g. ...macroenabled.12), which are still rejected today:

-  const matches = propertyValue.match(/^data:([A-Za-z.+/-]+);base64,(.+)$/);
+  const matches = propertyValue.match(/^data:([A-Za-z0-9.+/-]+);base64,(.+)$/);

This would broaden compatibility without changing the rest of the logic.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb38688 and 02e0935.

📒 Files selected for processing (3)
  • packages/engine/src/lib/variables/processors/file.ts (1 hunks)
  • packages/engine/src/lib/variables/props-processor.ts (2 hunks)
  • packages/engine/test/services/props-processor.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)

**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses: if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g., user-profile.ts)
Prefer const over let or var in TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript

Files:

  • packages/engine/src/lib/variables/processors/file.ts
  • packages/engine/src/lib/variables/props-processor.ts
  • packages/engine/test/services/props-processor.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)

**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript

Files:

  • packages/engine/src/lib/variables/processors/file.ts
  • packages/engine/src/lib/variables/props-processor.ts
  • packages/engine/test/services/props-processor.test.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)

**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios with qs package for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Use cn utility to group tailwind classnames in React components

Files:

  • packages/engine/src/lib/variables/processors/file.ts
  • packages/engine/src/lib/variables/props-processor.ts
  • packages/engine/test/services/props-processor.test.ts
🧬 Code graph analysis (1)
packages/engine/src/lib/variables/props-processor.ts (1)
packages/blocks/framework/src/lib/property/index.ts (1)
  • WorkflowFile (39-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build Engine Image for amd64
  • GitHub Check: Build Engine Image for arm64
  • GitHub Check: Build App Image for arm64
  • GitHub Check: Build App Image for amd64
  • GitHub Check: Test Server API
🔇 Additional comments (2)
packages/engine/test/services/props-processor.test.ts (1)

11-32: Distinct displayName prevents future FILE error-key collisions

Renaming the base64WithMime property’s displayName to 'Base64WithMime' keeps its error key distinct from the 'Base64' property, so if both ever fail validation they won’t overwrite each other in the errors map (which is keyed by displayName). Current expectations (only Base64 error) remain correct since base64WithMime is valid here.

packages/engine/src/lib/variables/props-processor.ts (1)

1-11: Verify WorkflowFile import consistency across all construction sites

Switching the FILE schema from a generic record to z.instanceof(WorkflowFile, ...) tightens validation so only true WorkflowFile instances pass validation, preventing arbitrary objects from being treated as valid files. This is a solid improvement if all code paths that populate FILE props use the same WorkflowFile class from @openops/blocks-framework.

However, if different modules construct WorkflowFile instances without importing from the same source, or if test code manually creates objects, the instanceof check will fail even for structurally identical objects. Confirm that every construction site—including apFileUtils.readWorkflowFile and any test/block code that creates file values—imports and uses WorkflowFile from @openops/blocks-framework.

@rashmi-sy rashmi-sy changed the title Fix schema for PropertyType.File and pattern matcher for mimetype to … Fix file uploads Dec 3, 2025
Copy link
Copy Markdown
Contributor

@maor-rozenfeld maor-rozenfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@maor-rozenfeld maor-rozenfeld merged commit 72e61ee into openops-cloud:main Dec 10, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants