Skip to content

Conversation

@lianwenke
Copy link

What problem does this PR solve? #7095

  • This PR addresses a critical bug where passing an empty kb_id ("") to the /rm endpoint could result in deletion of all knowledgebases owned by the current user.

  • Previously, the validate_request decorator did not reject an empty string, allowing the request to proceed.

  • As a result, the deletion logic would run KnowledgebaseService.query(created_by=current_user.id, id=""), which could unintentionally match and delete all entries.

Type of change

  • [ √] Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. 🐞 bug Something isn't working, pull request that fix bug. labels Apr 17, 2025
@KevinHuSh KevinHuSh added the ci Continue Integration label Apr 17, 2025
@KevinHuSh
Copy link
Collaborator

CI failure. Do you have any clue what happens?

@lianwenke
Copy link
Author

lianwenke commented Apr 22, 2025

@KevinHuSh
The decorator function @validate_request(...) is validating request inputs, and it specifically checks for required arguments. Your test setup is triggering the following error repeatedly:

Exception: required argument are missing or empty: asr_id;

This is because asr_id is expected in the request payload (request.json or request.form), but it's either:

  • not provided,
  • or is an empty string.
    Check this parameter is necessary
    image

@KevinHuSh
Copy link
Collaborator

Appreciations!
After our evaluation, this feature is not going to be merged into main branch.

@KevinHuSh KevinHuSh closed this Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working, pull request that fix bug. ci Continue Integration size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants