🛡️ Sentinel: Add input length limits to prevent DoS#34
🛡️ Sentinel: Add input length limits to prevent DoS#34
Conversation
- Add MAX_INPUT_LENGTHS to constants - Enforce maxLength in WelcomeScreen - Add character counters to UI - Fix missing script tag in index.html - Add tests for limits and truncation Co-authored-by: tblakex01 <17657984+tblakex01@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe pull request introduces input length validation across the application by defining a MAX_INPUT_LENGTHS constant (with limits: name 50, systemInstruction 2000, greeting 500, description 100), applying these via maxLength attributes in WelcomeScreen component with character counters, updating corresponding tests, and adding documentation about a DoS vulnerability. An index.html script module tag is also added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Reviewer's GuideAdds centralized maximum length constraints for configurable persona fields and wires them into the welcome screen inputs, updating tests and documentation, while also fixing the missing frontend entry script in index.html. Class diagram for WelcomeScreen and MAX_INPUT_LENGTHS usageclassDiagram
class WelcomeScreenProps {
+onStartCall(config PersonaConfig) void
}
class PersonaConfig {
+name string
+systemInstruction string
+greeting string
+voice VoiceName
}
class VoiceName {
}
class Constants {
<<module>>
+MAX_INPUT_LENGTHS name:number, systemInstruction:number, greeting:number, description:number
+VOICE_NAMES VoiceName[]
+PERSONA_PRESETS PersonaConfig[]
}
class WelcomeScreen {
<<ReactComponent>>
-customConfig PersonaConfig
-handleConfigChange(field string, value string) void
}
WelcomeScreen ..> WelcomeScreenProps : uses
WelcomeScreen ..> PersonaConfig : manages
WelcomeScreen ..> VoiceName : selects
WelcomeScreen ..> Constants : imports
Constants --> VoiceName : defines
Constants --> PersonaConfig : presets
Architecture/flow diagram for index.html entry and React bootstrapflowchart LR
Browser["Browser loads index_html"] --> HTMLBody["<div id=root>"]
HTMLBody --> ScriptTag["<script type=module src=/index_tsx>"]
ScriptTag --> EntryModule[index_tsx]
EntryModule --> ReactRoot["ReactDOM_createRoot(root)"]
ReactRoot --> WelcomeScreenComponent["Render WelcomeScreen"]
WelcomeScreenComponent --> Inputs["Name/SystemInstructions/Greeting inputs with maxLength from MAX_INPUT_LENGTHS"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
MAX_INPUT_LENGTHS.descriptionentry is defined but never used; either wire it up to the corresponding input or remove it to avoid dead configuration. - The new label + counter + input/textarea pattern for Name, System Instructions, and Greeting is duplicated; consider extracting a small reusable component to keep the markup and behavior consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `MAX_INPUT_LENGTHS.description` entry is defined but never used; either wire it up to the corresponding input or remove it to avoid dead configuration.
- The new label + counter + input/textarea pattern for Name, System Instructions, and Greeting is duplicated; consider extracting a small reusable component to keep the markup and behavior consistent and easier to maintain.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.jules/sentinel.md:
- Line 7: Update the compound modifier “unlimited length strings” to the
hyphenated form “unlimited-length strings” wherever that exact phrase appears
(the sentence describing the vulnerability for Name, Greeting, and System
Instructions); ensure spacing and punctuation remain correct and run a quick
proofread to confirm no other similar compound modifiers need hyphenation.
🧹 Nitpick comments (2)
components/WelcomeScreen.test.tsx (1)
5-5: Add system-instruction truncation coverage for symmetry.Now that name/greeting assert truncation via
MAX_INPUT_LENGTHS, consider updating the system-instruction test to exceed the limit and assert truncation as well.♻️ Proposed test tweak
- const longInstructions = 'Instructions '.repeat(100); + const longInstructions = 'I'.repeat(MAX_INPUT_LENGTHS.systemInstruction + 50); await user.clear(instructionsTextarea); await user.paste(longInstructions); - expect((instructionsTextarea as HTMLTextAreaElement).value).toBe(longInstructions); + const expectedInstructions = longInstructions.slice(0, MAX_INPUT_LENGTHS.systemInstruction); + expect((instructionsTextarea as HTMLTextAreaElement).value).toBe(expectedInstructions);Also applies to: 419-428, 457-457
components/WelcomeScreen.tsx (1)
3-3: Clamp config values before storing/submitting (defense in depth).
maxLengthonly limits user input; presets or future programmatic updates can still set over-limit strings and get sent viaonStartCall. Consider clamping in the handlers (and before submit) to keep the limit invariant.♻️ Suggested implementation
const [customConfig, setCustomConfig] = useState<PersonaConfig>(PERSONA_PRESETS[0]); + + const clampConfig = (config: PersonaConfig): PersonaConfig => ({ + ...config, + name: config.name.slice(0, MAX_INPUT_LENGTHS.name), + systemInstruction: config.systemInstruction.slice(0, MAX_INPUT_LENGTHS.systemInstruction), + greeting: config.greeting.slice(0, MAX_INPUT_LENGTHS.greeting), + }); const handlePresetChange = (presetId: string) => { setSelectedPresetId(presetId); if (presetId === 'custom') return; const preset = PERSONA_PRESETS.find(p => p.id === presetId); if (preset) { - setCustomConfig(preset); + setCustomConfig(clampConfig(preset)); } }; const handleConfigChange = (field: keyof PersonaConfig, value: string) => { - setCustomConfig(prev => ({ - ...prev, - [field]: value - })); + setCustomConfig(prev => clampConfig({ + ...prev, + [field]: value + })); setSelectedPresetId('custom'); }; ... - onClick={() => onStartCall(customConfig)} + onClick={() => onStartCall(clampConfig(customConfig))}Also applies to: 15-33, 79-90, 106-118, 120-132, 138-140
| **Prevention:** Use a sanitized logger wrapper instead of raw console methods. Add CSP meta tags. | ||
|
|
||
| ## 2026-01-28 - Unbounded User Inputs | ||
| **Vulnerability:** The application accepted unlimited length strings for Name, Greeting, and System Instructions, posing a Denial of Service (DoS) risk. |
There was a problem hiding this comment.
Minor grammar: hyphenate the compound modifier.
Line 7 reads “unlimited length strings”; standard style is “unlimited-length strings.”
✏️ Proposed fix
-**Vulnerability:** The application accepted unlimited length strings for Name, Greeting, and System Instructions, posing a Denial of Service (DoS) risk.
+**Vulnerability:** The application accepted unlimited-length strings for Name, Greeting, and System Instructions, posing a Denial of Service (DoS) risk.📝 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.
| **Vulnerability:** The application accepted unlimited length strings for Name, Greeting, and System Instructions, posing a Denial of Service (DoS) risk. | |
| **Vulnerability:** The application accepted unlimited-length strings for Name, Greeting, and System Instructions, posing a Denial of Service (DoS) risk. |
🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: Use a hyphen to join words.
Context: ...ty:** The application accepted unlimited length strings for Name, Greeting, and S...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In @.jules/sentinel.md at line 7, Update the compound modifier “unlimited length
strings” to the hyphenated form “unlimited-length strings” wherever that exact
phrase appears (the sentence describing the vulnerability for Name, Greeting,
and System Instructions); ensure spacing and punctuation remain correct and run
a quick proofread to confirm no other similar compound modifiers need
hyphenation.
1 participant
🚨 Severity: MEDIUM
💡 Vulnerability: Missing input length limits on user configuration fields (Name, System Instructions, Greeting).
🎯 Impact: Potential Denial of Service (DoS) or performance degradation if massive strings are input.
🔧 Fix:
MAX_INPUT_LENGTHStoconstants.ts.maxLengthon inputs inWelcomeScreen.tsx.index.html.✅ Verification:
npm testpassed.PR created automatically by Jules for task 4003401425753474456 started by @tblakex01
Summary by Sourcery
Enforce bounded user input lengths in the welcome screen configuration and wire up the app entry point script.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.