Skip to content

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Jul 18, 2025

Description

Validate interaction service input matches what is expected:

  • Required inputs have a value
  • Number inputs are a number
  • Boolean inputs are a boolean
  • Choice inputs are one of the options
  • Give text inputs a configurable max length. Default value of 8000 should be large enough for any normal usage. Could add configuration for a custom length in the future

The dashboard or CLI shouldn't send these values, UI logic should prevent that from happening, but server should double check.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@JamesNK JamesNK added this to the 9.4 milestone Jul 18, 2025
@JamesNK JamesNK requested review from davidfowl, adamint and Copilot July 18, 2025 07:17
@JamesNK JamesNK requested a review from mitchdenny as a code owner July 18, 2025 07:17
@JamesNK JamesNK added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jul 18, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive server-side validation for interaction service inputs to ensure data integrity and provide clear error messages to users. The validation covers all input types with appropriate type checking and constraint enforcement.

  • Validates required inputs, data types (number, boolean), choice options, and text length limits
  • Adds server-side validation that runs before custom validation callbacks
  • Implements a maximum text length of 8000 characters for text inputs with corresponding UI enforcement

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Aspire.Hosting/InteractionService.cs Core validation logic for all input types with error handling
src/Aspire.Hosting/IInteractionService.cs Defines maximum text length constant for validation
src/Aspire.Dashboard/Components/Dialogs/InteractionsInputDialog.razor.cs Dashboard constant for text length matching server validation
src/Aspire.Dashboard/Components/Dialogs/InteractionsInputDialog.razor UI enforcement of maximum text length on input fields
tests/Aspire.Hosting.Tests/InteractionServiceTests.cs Comprehensive test coverage for all validation scenarios


public partial class InteractionsInputDialog
{
internal const int MaxTextLength = 8000;
Copy link
Preview

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

The MaxTextLength constant is duplicated between InteractionInput and InteractionsInputDialog. Consider referencing the constant from InteractionInput instead of duplicating it to maintain a single source of truth.

Copilot uses AI. Check for mistakes.

@davidfowl
Copy link
Member

If this makes back porting harder then we shouldn’t merge this or we should backport it

@JamesNK
Copy link
Member Author

JamesNK commented Jul 18, 2025

This is security hardening and should be in 9.4

@davidfowl
Copy link
Member

Could add configuration for a custom length in the future

Lets add this.

@JamesNK JamesNK force-pushed the jamesnk/interactionservice-servervalidation branch from bc9968f to c485449 Compare July 19, 2025 05:51
@JamesNK
Copy link
Member Author

JamesNK commented Jul 19, 2025

Could add configuration for a custom length in the future

Lets add this.

Done

@JamesNK JamesNK merged commit 261532e into main Jul 19, 2025
276 checks passed
@JamesNK JamesNK deleted the jamesnk/interactionservice-servervalidation branch July 19, 2025 09:43
@JamesNK
Copy link
Member Author

JamesNK commented Jul 19, 2025

/backport to release/9.4

Copy link
Contributor

Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16387420476

@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants