fix(DATAGO-121653): reject prompt imports with invalid command characters#1542
fix(DATAGO-121653): reject prompt imports with invalid command characters#1542solace-jkhalife wants to merge 6 commits into
Conversation
✅ FOSSA Guard: Licensing (
|
✅ FOSSA Guard: Vulnerability (
|
There was a problem hiding this comment.
Pull request overview
This PR prevents “invisible” prompts after import by validating the command/chat-shortcut field against the same allowed character set used by prompt-list responses, rejecting invalid values both server-side (HTTP 400) and client-side (form validation).
Changes:
- Backend: validate imported
commandagainst^[a-zA-Z0-9_-]+$whenpreserveCommandis enabled, returning 400 on mismatch. - Frontend: add command regex validation to the import dialog and gate the Import action on overall form validity; adjust RHF
setValuecalls to trigger validation. - Tests: add backend integration tests for invalid commands + preserveCommand=false behavior; add frontend unit tests for dialog UX and button enablement.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/solace_agent_mesh/gateway/http_sse/routers/prompts.py |
Adds backend command pattern validation during prompt import. |
client/webui/frontend/src/lib/components/prompts/PromptImportDialog.tsx |
Adds client-side command regex validation, initial invalid-state handling, and Import button gating on form validity. |
client/webui/frontend/src/stories/Prompts/PromptImportDialog.test.tsx |
Adds unit tests covering invalid/valid command UI states and Import button enablement. |
tests/integration/apis/persistence/prompts/test_prompts_validation_and_auth.py |
Adds integration coverage for rejecting invalid commands on import and ignoring command when preserveCommand is false. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
lgh-solace
left a comment
There was a problem hiding this comment.
Looks good! Can you check out the below comments?
Please review and fix if confirmed
- The same
$-before-newline quirk likely affects the create/update path too. Python'sre(withoutre.MULTILINE) treats$as matching either end-of-string or right before a trailing\n, and Pydantic v2's stringpatternuses those semantics. The Field constraints at prompt_dto.py:69 and prompt_dto.py:93 still usepattern="^[a-zA-Z0-9_-]+$"— did you consider whetherPOST /groups/PATCH /groups/{id}with"command": "valid-cmd\n"slips through the same way the import path used to? A quick integration probe would confirm. If it does, the cleanest fix is afield_validatorusingre.fullmatchagainst a shared constant — which would also collapse the three regex copies.
Other areas to consider
- One regex, three copies.
re.fullmatchliteral at prompts.py:1257, Pydanticpatternat prompt_dto.py:69 / :93, andCOMMAND_PATTERNat PromptImportDialog.tsx:12. The new code-comment already calls out the coupling ("must match PromptGroupResponse") — extracting one Python constant + one FE constant alongsidePROMPT_FIELD_LIMITSwould prevent the next drift. promptImportCommandSchemais unreferenced and duplicates the dialog's form schema. prompt-import.schema.ts:62-64 is dead code, and the regex change was made to the inline schema in the dialog instead. Worth deleting it or moving the dialog'spromptImportFormSchemainto the schema file so the rule lives in one place.- Backend truncates before it validates at prompts.py:1251-1257 — a 51-char command with an invalid char at position 51 will be sliced off and pass silently. Worth either swapping the order or adding a
# whynote. - PromptImportDialog.test.tsx:11-21 polyfills
Blob.prototype.textper-file. If jsdom really lacks it in this version, this belongs invitest.setup.tsonce rather than copied into every new dialog test —PromptBuilderChat.test.tsxdoesn't need it, which suggests it may not be necessary here either.
11).
Co-authored-by: Copilot Autofix powered by AI <[email protected]> Signed-off-by: John Khalife <[email protected]>




What is the purpose of this change?
When importing a prompt with a
commandfield containing characters outside[a-zA-Z0-9_-](e.g. spaces, dots,@), the import would silently succeed but the resulting row would be dropped from all list responses — making the prompt invisible after import. The fix validates the command pattern at the API layer and in the import dialog UI before the request is submitted.How was this change implemented?
routers/prompts.py): Added a regex check against^[a-zA-Z0-9_-]+$on thecommandfield in theimport_promptendpoint. Returns HTTP 400 with the offending value in the error message if validation fails. Only applied whenpreserve_command=true; ignored otherwise.PromptImportDialog.tsx): AddedCOMMAND_PATTERNregex to the Zod form schema for the command field. AddedinitialCommandInvalidstate so that files with an invalid command immediately open the field in editable mode (same UX as conflict handling). AddedisValidto theisEnabledgate on the Import button. FixedsetValuecalls to passshouldValidate: trueso react-hook-form recalculatesisValidafter programmatic population.preserve_command=false. Frontend unit tests covering editable input appearance, button disabled state, button re-enable after correction, and read-only display for valid commands.Key Design Decisions
The validation pattern (
^[a-zA-Z0-9_-]+$) mirrors the existingPromptGroupResponsefilter — the same characters that cause silent drops from list queries.How was this change tested?
PromptImportDialog.test.tsx(all passing)test_prompts_validation_and_auth.py(all passing on SQLite)Is there anything the reviewers should focus on/be aware of?