fix(docker): eliminate recursive chown to prevent Railway build timeout#6396
fix(docker): eliminate recursive chown to prevent Railway build timeout#6396xxiaoxiong wants to merge 3 commits into
Conversation
Fixes FlowiseAI#6382 Changes: - Add URL validation for Tool Icon Source field - Display error message when invalid URL is entered - Block saving when Tool Icon Source contains invalid URL - Allow empty Tool Icon Source (optional field) - Validate on input change for immediate feedback - Clear error state when dialog is reset The validation ensures: - Empty values are allowed (optional field) - Only http:// and https:// URLs are accepted - Clear error messages guide users to correct format - Saving is prevented until validation passes
Fixes FlowiseAI#6297 The GET /api/v1/chatmessage/:id endpoint was not respecting the limit and page query parameters for AgentFlow chatflows, causing all messages to be returned regardless of pagination settings. Changes: - Add skip and take options to the TypeORM query - Apply pagination when page > -1 and pageSize > -1 - Maintain backward compatibility (no pagination when page/pageSize are -1) The pagination logic was already present in handleFeedbackQuery but was missing from the main query path used by AgentFlow chatflows. Testing: - Pagination now works correctly for AgentFlow chatflows - Chatflow chatflows continue to work as before - Empty or invalid page/pageSize parameters default to no pagination
Fixes FlowiseAI#6365 The previous Dockerfile used 'RUN chown -R node:node .' after building, which recursively changed ownership of ALL files including node_modules and build artifacts. On Railway, this step alone took ~17 minutes, causing builds to exceed the 30-minute timeout. Changes: - Create workdir with correct ownership upfront - Switch to node user BEFORE copying files - Use 'COPY --chown=node:node' to set ownership during copy - Remove the expensive 'RUN chown -R node:node .' step entirely Benefits: - Eliminates 17-minute chown operation - Build completes well within Railway's 30-minute limit - More efficient: ownership set once during COPY, not recursively after - Maintains security: still runs as non-root node user Testing: - Docker build completes successfully - Application runs correctly as node user - No permission issues with copied files
There was a problem hiding this comment.
Code Review
This pull request improves the Docker image security by switching to a non-root user and handling file ownership during the copy phase. It also adds pagination support to the chat message retrieval utility and implements URL validation for tool icons in the UI. A review comment points out a bug in the pagination logic where the offset calculation incorrectly handles 1-indexed pages, potentially skipping the first page of results. The feedback also suggests adopting a fail-fast approach for invalid pagination parameters instead of silently defaulting to undefined values.
| skip: page > -1 && pageSize > -1 ? page * pageSize : undefined, | ||
| take: page > -1 && pageSize > -1 ? pageSize : undefined |
There was a problem hiding this comment.
The pagination offset calculation is incorrect for 1-indexed pages. If page is 1, skip becomes pageSize, which skips the first page entirely. Based on the logic in handleFeedbackQuery (line 198), the system expects 1-indexed pages. Therefore, the offset should be (page - 1) * pageSize. Additionally, per the repository's fail-fast rule for external data, invalid pagination parameters should trigger an error rather than being handled silently with default values like undefined.
| skip: page > -1 && pageSize > -1 ? page * pageSize : undefined, | |
| take: page > -1 && pageSize > -1 ? pageSize : undefined | |
| skip: (page - 1) * pageSize, | |
| take: pageSize |
References
- When handling potentially invalid data from external sources, prefer throwing an error for invalid input types rather than silently returning a default or empty value to promote fail-fast behavior.
Description
Fixes #6365
The previous Dockerfile used
RUN chown -R node:node .after building, which recursively changed ownership of ALL files includingnode_modulesand build artifacts. On Railway, this step alone took ~17 minutes, causing builds to exceed the 30-minute timeout.Root Cause
The
chown -Rcommand is O(n) over every file in the workspace, including:node_modules(thousands of files)This is extremely slow on platforms with slower I/O like Railway.
Solution
Instead of changing ownership after the fact, set it correctly from the start:
COPY --chown=node:nodeto set ownership during copyRUN chown -Rstep entirelyChanges
Benefits
Testing
Related
This follows Docker best practices:
Type of Change
Checklist