🛡️ Sentinel: Enforce input length limits to prevent DoS#35
Open
🛡️ Sentinel: Enforce input length limits to prevent DoS#35
Conversation
- Added `MAX_INPUT_LENGTHS` to `constants.ts` to define safe limits for user inputs. - Updated `WelcomeScreen.tsx` to enforce these limits on Name, System Instructions, and Greeting fields. - Added visual character counters to `WelcomeScreen`. - Fixed a bug in `index.html` where the entry script was missing. - Updated `WelcomeScreen.test.tsx` to verify truncation logic. This mitigates potential DoS risks from large payloads and improves UX. Co-authored-by: tblakex01 <17657984+tblakex01@users.noreply.github.com>
Contributor
|
👋 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. |
Reviewer's GuideAdds centralized maximum length limits for persona-related inputs on the Welcome screen, enforces them both in React state and HTML attributes with user-visible character counters, updates tests and security documentation, and restores the main script tag in index.html so the app loads correctly. Sequence diagram for enforcing input length on WelcomeScreensequenceDiagram
actor User
participant NameInput
participant WelcomeScreen
participant MAX_INPUT_LENGTHS
participant Backend
User->>NameInput: Type or paste very_long_name
NameInput->>WelcomeScreen: onChange(value = very_long_name)
WelcomeScreen->>WelcomeScreen: handleConfigChange(field = name, value)
WelcomeScreen->>MAX_INPUT_LENGTHS: Read MAX_INPUT_LENGTHS.name
alt value.length > MAX_INPUT_LENGTHS.name
WelcomeScreen->>WelcomeScreen: finalValue = value.slice(0, MAX_INPUT_LENGTHS.name)
else value.length <= MAX_INPUT_LENGTHS.name
WelcomeScreen->>WelcomeScreen: finalValue = value
end
WelcomeScreen->>WelcomeScreen: setCustomConfig with name = finalValue
WelcomeScreen-->>NameInput: Rerender with value = finalValue
User->>WelcomeScreen: Click StartCall
WelcomeScreen->>Backend: onStartCall(customConfig with truncated name)
Backend-->>WelcomeScreen: Call started with bounded input
WelcomeScreen-->>User: Call UI shown
Updated class diagram for WelcomeScreen and constantsclassDiagram
class WelcomeScreenProps {
+onStartCall(config PersonaConfig) void
}
class PersonaConfig {
+name string
+description string
+systemInstruction string
+greeting string
+voice VoiceName
}
class WelcomeScreen {
-customConfig PersonaConfig
-selectedPresetId string
-selectedVoice VoiceName
+WelcomeScreen(props WelcomeScreenProps)
+handleConfigChange(field keyof_PersonaConfig, value string) void
+handlePresetSelect(presetId string) void
+handleStartCall() void
}
class MAX_INPUT_LENGTHS {
<<constant_object>>
+name number
+description number
+systemInstruction number
+greeting number
}
class PERSONA_PRESETS {
<<constant_array>>
+items PersonaConfig_with_id[]
}
class PersonaConfig_with_id {
+id string
+name string
+description string
+systemInstruction string
+greeting string
+voice VoiceName
}
class VOICE_NAMES {
<<constant_array>>
+items VoiceName[]
}
class VoiceName {
<<type_alias>>
}
WelcomeScreen --> WelcomeScreenProps : uses_props
WelcomeScreen *-- PersonaConfig : manages_customConfig
WelcomeScreen ..> MAX_INPUT_LENGTHS : enforces_length_limits
WelcomeScreen ..> PERSONA_PRESETS : uses_default_presets
WelcomeScreen ..> VOICE_NAMES : uses_allowed_voices
PERSONA_PRESETS *-- PersonaConfig_with_id : contains
PersonaConfig_with_id --> PersonaConfig : extends_shape
VOICE_NAMES o-- VoiceName : elements_of_type
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe changes implement input length validation for the WelcomeScreen component by introducing a MAX_INPUT_LENGTHS constant, truncating values in the component logic, adding character counters to the UI, and updating tests to verify the new constraints. A security documentation entry records the vulnerability that prompted these changes. 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
🧪 Generate unit tests (beta)
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}
Contributor
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.
![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
handleConfigChangetruncation logic could be simplified by looking up the max length fromMAX_INPUT_LENGTHS[field](with appropriate typing) instead of a three-branchif/elsechain, which will make it easier to extend when new fields are added. MAX_INPUT_LENGTHS.descriptionis defined but not used anywhere in this PR; consider either wiring it into the relevant input or removing it to avoid confusion about dead configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `handleConfigChange` truncation logic could be simplified by looking up the max length from `MAX_INPUT_LENGTHS[field]` (with appropriate typing) instead of a three-branch `if/else` chain, which will make it easier to extend when new fields are added.
- `MAX_INPUT_LENGTHS.description` is defined but not used anywhere in this PR; consider either wiring it into the relevant input or removing it to avoid confusion about dead configuration.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛡️ Sentinel Security Update
Severity: MEDIUM (DoS Risk)
Vulnerability:
The
WelcomeScreencomponent lacked input validation, allowing users to enter arbitrarily large strings for Name, System Instructions, and Greeting. This could potentially be used to send excessive data to the backend or cause performance issues.Fix:
MAX_INPUT_LENGTHSinconstants.ts.WelcomeScreen.tsxstate updates.maxLengthattributes to input elements.12/50) for better user feedback.<script type="module" src="/index.tsx"></script>inindex.htmlwhich was preventing the app from loading during verification.Verification:
WelcomeScreen.test.tsxto assert that inputs are truncated to the defined limits.npm testsuccessfully (all 328 tests passed).PR created automatically by Jules for task 7113029068086227799 started by @tblakex01
Summary by Sourcery
Enforce input length limits and improve feedback for the Welcome screen to mitigate potential DoS risks and restore the main app script in the HTML entrypoint.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Documentation