Skip to content

supprt restrict file types in scenario#8283

Open
smallst wants to merge 3 commits intomasterfrom
yuqiang/restrict-file-type
Open

supprt restrict file types in scenario#8283
smallst wants to merge 3 commits intomasterfrom
yuqiang/restrict-file-type

Conversation

@smallst
Copy link
Copy Markdown
Contributor

@smallst smallst commented Apr 10, 2026

fix ENG-2411

Summary by CodeRabbit

  • New Features

    • File upload restrictions now vary by AI model; the UI shows allowed file types per model.
    • Scenarios can now specify allowed file types for uploads.
  • Localization

    • English copy updated to prefix an access-denied message.
    • Added a translation key for the "Allowed file types" label in the scenario details panel.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

Adds model-specific allowed file-type configuration, exposes it via a new exported constant, extends the AIScenario schema with an allowedFileTypes array, and adds a corresponding English localization key for the UI label.

Changes

Cohort / File(s) Summary
File Type Restrictions Configuration
app/core/constants.js
Added exported FILE_TYPES_BY_MODEL (array of { pattern, types }) mapping model-name prefixes to allowed MIME types; adjusted trailing commas in multiple literals; included FILE_TYPES_BY_MODEL in module.exports.
Schema Enhancement
app/schemas/models/ai_scenario.schema.js
Added allowedFileTypes property to AIScenarioSchema as an array of string to record permitted MIME types for uploads.
Localization Support
app/locale/en.js
Updated home_campaign_redirect_student text to add “Access denied” prefix and added ScenarioDetailsPanel_allowed-file-types translation key for the UI label.

Sequence Diagram(s)

(Skipped — changes are configuration/schema/localization only and do not introduce multi-component sequential flows.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • mrfinch

Poem

🐰 I hopped in code to map each type,
gpt, claude, gemini lined up right,
MIME rules set with tidy art,
Small changes, big peace of heart —
Hop, commit, and into the night ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title contains a typo ('supprt' instead of 'support') and is vague about the actual implementation, which adds file type restriction constants, schema properties, and translations across three files. Correct the typo and make the title more specific. Consider: 'Add file type restriction support for AI scenario uploads' or similar to better reflect the multi-file changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yuqiang/restrict-file-type

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.

🔧 ast-grep (0.42.1)
app/locale/en.js

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.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@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: 1

🧹 Nitpick comments (1)
app/schemas/models/ai_scenario.schema.js (1)

104-111: Harden allowedFileTypes validation at schema level.

Line 104 currently accepts any string, so invalid MIME values can be saved and later break filtering behavior. Add basic MIME-shape validation and dedupe guard.

Proposed diff
   allowedFileTypes: {
     title: 'Allowed Types of Uploaded File',
     description: 'MIME types of files that can be uploaded',
     type: 'array',
+    uniqueItems: true,
     items: {
       type: 'string',
+      pattern: '^([A-Za-z0-9!#$&^_.+-]+|\\*)/([A-Za-z0-9!#$&^_.+-]+|\\*)$',
     },
   },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/schemas/models/ai_scenario.schema.js` around lines 104 - 111, The
allowedFileTypes array currently permits any string; update the allowedFileTypes
schema to enforce MIME-shape validation and deduplication by adding uniqueItems:
true and replacing the simple items type with an object that has type: "string"
and a pattern that matches basic MIME types (e.g. token/token with allowed
characters like letters, digits, ".", "+", "-" ), and optionally set a sensible
minItems if required; target the allowedFileTypes field in the schema to make
these changes so saved values conform to expected MIME format and duplicates are
prevented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/locale/en.js`:
- Line 6402: The label string for ScenarioDetailsPanel_allowed-file-types has
grammatical errors; update its value to a clearer phrase such as "Allowed file
types users can upload" (or simply "Allowed file types") by replacing the
existing text for the ScenarioDetailsPanel_allowed-file-types key in
app/locale/en.js with the corrected wording.

---

Nitpick comments:
In `@app/schemas/models/ai_scenario.schema.js`:
- Around line 104-111: The allowedFileTypes array currently permits any string;
update the allowedFileTypes schema to enforce MIME-shape validation and
deduplication by adding uniqueItems: true and replacing the simple items type
with an object that has type: "string" and a pattern that matches basic MIME
types (e.g. token/token with allowed characters like letters, digits, ".", "+",
"-" ), and optionally set a sensible minItems if required; target the
allowedFileTypes field in the schema to make these changes so saved values
conform to expected MIME format and duplicates are prevented.
🪄 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

Run ID: 2ec84cdd-51dc-4296-a33d-3b60fe1de40c

📥 Commits

Reviewing files that changed from the base of the PR and between 5f5b6d3 and 7647e8d.

📒 Files selected for processing (3)
  • app/core/constants.js
  • app/locale/en.js
  • app/schemas/models/ai_scenario.schema.js

Comment thread app/locale/en.js
'ScenarioDetailsPanel_save-scenario': 'Save Scenario',
'ScenarioDetailsPanel_scenario-settings': 'Scenario settings',
'ScenarioDetailsPanel_allow-user-upload-file': 'Allow user to upload file',
'ScenarioDetailsPanel_allowed-file-types': 'Allowed Files Types user can upload',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix grammar in the new UI label.

The string has wording issues (Files Types, singular user). Please update it to a clearer label, e.g. "Allowed file types users can upload" (or shorter: "Allowed file types").

✏️ Suggested text fix
-      'ScenarioDetailsPanel_allowed-file-types': 'Allowed Files Types user can upload',
+      'ScenarioDetailsPanel_allowed-file-types': 'Allowed file types users can upload',
📝 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.

Suggested change
'ScenarioDetailsPanel_allowed-file-types': 'Allowed Files Types user can upload',
'ScenarioDetailsPanel_allowed-file-types': 'Allowed file types users can upload',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/locale/en.js` at line 6402, The label string for
ScenarioDetailsPanel_allowed-file-types has grammatical errors; update its value
to a clearer phrase such as "Allowed file types users can upload" (or simply
"Allowed file types") by replacing the existing text for the
ScenarioDetailsPanel_allowed-file-types key in app/locale/en.js with the
corrected wording.

Comment thread app/core/constants.js

const USER_CREDIT_HACKSTACK_KEY = 'HACKSTACK_QUERY'

const FILE_TYPES_BY_MODEL = [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if we want to restrict this, I would rather have it as static strings here or in AI folder than store in AIScenario so that we don't need to run a script for old scenarios

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

put in ai will make server cannot access it. i first want to double check in server side if uploaded file is in correct format(since in frontend user can anyway change the file types to . so override the limited file types. -- in file upload modal) while in current pr i didn't check that. so make this string in coco repo little strange.

Copy link
Copy Markdown
Contributor

@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.

♻️ Duplicate comments (1)
app/locale/en.js (1)

6403-6403: ⚠️ Potential issue | 🟡 Minor

Fix grammar in the new label text.

Line 6403 still has grammatical issues in a user-facing string.

✏️ Suggested text fix
-      'ScenarioDetailsPanel_allowed-file-types': 'Allowed Files Types user can upload',
+      'ScenarioDetailsPanel_allowed-file-types': 'Allowed file types users can upload',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/locale/en.js` at line 6403, Update the user-facing string for the i18n
key 'ScenarioDetailsPanel_allowed-file-types' in app/locale/en.js to correct
grammar; replace "Allowed Files Types user can upload" with a grammatically
correct variant such as "Allowed file types users can upload" (or "Allowed file
types a user can upload"/"Allowed file types that users can upload") so the
label reads naturally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@app/locale/en.js`:
- Line 6403: Update the user-facing string for the i18n key
'ScenarioDetailsPanel_allowed-file-types' in app/locale/en.js to correct
grammar; replace "Allowed Files Types user can upload" with a grammatically
correct variant such as "Allowed file types users can upload" (or "Allowed file
types a user can upload"/"Allowed file types that users can upload") so the
label reads naturally.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b516d5f-b39c-43d3-bd22-a02e160e4411

📥 Commits

Reviewing files that changed from the base of the PR and between 7647e8d and 563a874.

📒 Files selected for processing (1)
  • app/locale/en.js

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