🛡️ Sentinel: [MEDIUM] Fail securely on null inputs in Duck<T> API#53
🛡️ Sentinel: [MEDIUM] Fail securely on null inputs in Duck<T> API#53timonkrebs wants to merge 1 commit into
Conversation
Co-authored-by: timonkrebs <11026852+timonkrebs@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. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the NTypeForge public duck-typing API by failing fast and accurately when Duck<T> is invoked with a null receiver, aligning runtime behavior with secure input-validation expectations and documenting the lesson in the Sentinel log.
Changes:
- Added
ArgumentNullException.ThrowIfNull(instance)toDuck<T>(this object instance)to explicitly reject null inputs. - Updated
.jules/sentinel.mdwith a new entry documenting the null-input validation lesson and prevention guidance.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/NTypeForge/DuckExtensions.cs | Adds explicit null-guard to Duck<T> to throw ArgumentNullException instead of a misleading InvalidOperationException. |
| .jules/sentinel.md | Documents the vulnerability, learning, and prevention steps related to null input validation on public APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// </exception> | ||
| public static T Duck<T>(this object instance) where T : class | ||
| { | ||
| ArgumentNullException.ThrowIfNull(instance); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cb219e20a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// </exception> | ||
| public static T Duck<T>(this object instance) where T : class | ||
| { | ||
| ArgumentNullException.ThrowIfNull(instance); |
There was a problem hiding this comment.
This guard only runs in the runtime fallback, but the common source-generated Duck<T> path emitted by ProxyEmitter.EmitDuckMethod bypasses this method entirely. For a nullable concrete receiver that structurally matches an interface (for example Adder? a = null; a.Duck<ICalc>()), the generated extension still wraps the null receiver and returns a proxy that later fails with a NullReferenceException, so the public API does not consistently fail fast on null inputs unless the generator emits the same check.
Useful? React with 👍 / 👎.
🚨 Severity: MEDIUM
💡 Vulnerability: The public API
Duck<T>(this object instance)did not explicitly validate theinstanceinput for null values. If null was passed, it bypassed the type check and threw a misleadingInvalidOperationExceptionoriginally intended for missing generator proxies.🎯 Impact: Obscures root causes and fails to securely halt execution early, which could potentially lead to unexpected application state if the incorrect exception type is caught or misdiagnosed downstream.
🔧 Fix: Added explicit input validation (
ArgumentNullException.ThrowIfNull(instance)) at the start of theDuck<T>method to enforce failing securely and fast. Documented the learning in.jules/sentinel.md.✅ Verification: Ran
NTypeForge.Generator.Teststo verify no regressions. Manually verified in a test project thatDuck<T>correctly throwsArgumentNullExceptionsecurely on null input. Code review successfully approved.PR created automatically by Jules for task 7274913049103684126 started by @timonkrebs