Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 12, 2025

Problem

File uploads were being stored in /models/tmp within the module directory, which could be publicly accessible if the web server wasn't properly configured to block access. This created a security vulnerability where uploaded files could be accessed via HTTP before being processed or validated.

Solution

This PR implements two key security improvements:

1. Secure Upload Location

Files are now uploaded to the system temporary directory by default using getTempDirectory() & "/cbwire". This directory is typically not accessible via HTTP, protecting uploaded files until they are explicitly moved to permanent storage.

Configuration changes:

  • Added new uploadsStoragePath setting that uses the system temp directory for file uploads
  • Preserved existing storagePath setting for single-file component compilation (must remain in module directory for WireBox to instantiate components)

2. New store() Method

Added a store(path) method to FileUpload.cfc that allows developers to move files from temporary storage to a permanent location:

<cfoutput>
    <div>
        <h1>PDF Upload</h1>
        <input type="file" wire:model="pdfFile" />
        <button wire:click="submitUpload">Upload</button>
    </div>
</cfoutput>

<cfscript>
    // @startWire
    data = {
        "pdfFile": ""
    };
    
    function submitUpload() {
        data.pdfFile.store( "/a/safe/folder" );
    }
    // @endWire
</cfscript>

The store() method:

  • Moves files from temporary to permanent storage
  • Accepts both directory paths (uses original filename) and full file paths
  • Automatically creates destination directories if they don't exist
  • Updates internal metadata to track the new file location
  • Returns the absolute path to the stored file
  • Maintains metadata file in temp directory for upload tracking

Changes Made

  • ModuleConfig.cfc: Added uploadsStoragePath configuration setting
  • FileUpload.cfc: Added store() method and updated temp directory logic
  • CBWIREController.cfc: Updated file upload and preview handlers to use uploadsStoragePath
  • CBWIRE.cfc: Updated cleanup logic to handle both storage directories
  • Component.cfc: Fixed reset() method to handle properties that don't exist in initial state
  • Tests: Added 4 comprehensive tests for the new store() method and updated existing tests with improved file handling

Security Benefits

✅ Uploaded files no longer in publicly accessible directory by default
✅ Files are protected until explicitly moved to permanent storage via store()
✅ Server configuration no longer needs to block access to /models/tmp
✅ Follows security best practices for temporary file handling

Backward Compatibility

This change is fully backward compatible with no breaking changes:

  • Existing file upload functionality continues to work as before
  • Single-file component compilation is unaffected
  • New store() method is optional - existing code doesn't need to be updated immediately

Testing

Added comprehensive test coverage including:

  • Storing files to directory paths
  • Storing files to specific file paths
  • Verifying temporary storage path updates after store
  • Verifying destroy() works correctly after store()

Fixes #216

Original prompt

This section details on the original issue you should resolve

<issue_title>Make file uploads by default upload to the directory returned using the getTempDirectory() method and add .store() method to FileUpload.cfc</issue_title>
<issue_description>Currently file uploads are problematic because they are being uploaded to a folder that is publicly accessible if your server is not configured to block access. In CBWIRE version 5 we want to ensure that uploads by default are uploaded to the temporary directory provided by the CFML server using the getTempDirectory() BIF and we want to also add a store() method to the FileUpload.cfc object that will move the file from the temporary folder to wherever we have specified when calling store( path ).</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #216


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits October 12, 2025 01:51
…re() method

Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Copilot AI changed the title [WIP] Make file uploads default to temp directory and add store method Make file uploads use system temp directory by default and add store() method for permanent storage Oct 12, 2025
Copilot AI requested a review from grantcopley October 12, 2025 01:58
@grantcopley grantcopley marked this pull request as ready for review October 12, 2025 01:58
@grantcopley
Copy link
Collaborator

@copilot I've pushed up some changes

@grantcopley
Copy link
Collaborator

grantcopley commented Oct 12, 2025

@copilot We need to adjust your example for the store() method. Here's what it should look like. BTW this is already working fine, but you need to update the example in the PR.

<cfoutput>
    <div>
        <h1>PDF Upload</h1>
        <input type="file" wire:model="pdfFile" />
        <button wire:click="submitUpload">Upload</button>
    </div>
</cfoutput>

<cfscript>
    // @startWire
    data = {
        "pdfFile": ""
    };

    function submitUpload() {
        data.pdfFile.store( "/a/safe/folder" );
    }
    // @endWire
</cfscript>

Copy link
Contributor Author

Copilot AI commented Oct 12, 2025

@copilot We need to adjust your example for the store() method. Here's what it should look like. BTW this is already working fine, but you need to update the example in the PR.

<cfoutput>

...

The code block in your comment appears to be empty. Could you provide the example code that should replace the current store() method example in the PR description?

@grantcopley
Copy link
Collaborator

@copilot

<cfoutput>
    <div>
        <h1>PDF Upload</h1>
        <input type="file" wire:model="pdfFile" />
        <button wire:click="submitUpload">Upload</button>
    </div>
</cfoutput>

<cfscript>
    // @startWire
    data = {
        "pdfFile": ""
    };

    function submitUpload() {
        data.pdfFile.store( "/a/safe/folder" );
    }
    // @endWire
</cfscript>

Copy link
Contributor Author

Copilot AI commented Oct 12, 2025

@copilot

<cfoutput>

...

The code block still appears empty. Could you paste the example as plain text or share it in a different format? I'm unable to see the content of the code block you're trying to share.

@grantcopley
Copy link
Collaborator

@copilot
CleanShot 2025-10-12 at 07 58 23@2x

Copy link
Contributor Author

Copilot AI commented Oct 12, 2025

@copilot
CleanShot 2025-10-12 at 07 58 23@2x

Got it! I can see the example now. The PR description should be updated to show the store() method being called directly on the file upload data property in a CBWIRE component context like this:

<cfoutput>
    <div>
        <h1>PDF Upload</h1>
        <input type="file" wire:model="pdfFile" />
        <button wire:click="submitUpload">Upload</button>
    </div>
</cfoutput>

<cfscript>
    // @startWire
    data = {
        "pdfFile": ""
    };
    
    function submitUpload() {
        data.pdfFile.store( "/a/safe/folder" );
    }
    // @endWire
</cfscript>

This is a much clearer example showing the actual CBWIRE single-file component syntax with the file upload and how to use store() on the data property. I don't have permissions to update the PR description directly, but you can update it with this corrected example.

@grantcopley grantcopley merged commit 18741d4 into next Oct 12, 2025
5 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