fix(editor): prevent field name in field value autocompletion COMPASS-7297#7924
fix(editor): prevent field name in field value autocompletion COMPASS-7297#7924
Conversation
| 'expr:*', | ||
| 'conv', | ||
| 'bson', | ||
| 'bson-legacy-uuid', |
There was a problem hiding this comment.
drive-by. I totally missed this in COMPASS-9690 🙈
There was a problem hiding this comment.
Pull request overview
This PR updates the Compass editor’s MongoDB query autocomplete to be context-aware so it can offer different suggestions when completing a field name vs completing a value (addressing COMPASS-7297).
Changes:
- Implement context-sensitive query completions by detecting whether the cursor is in a property value and filtering suggestions accordingly.
- Centralize MongoDB→CodeMirror completion mapping utilities and add a helper for detecting “value position”.
- Extend stage autocompletion to include
bson-legacy-uuidand update/expand tests for the new behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-editor/src/codemirror/utils.ts | Adds isPropertyValue and moves completion-mapping helper into shared utilities. |
| packages/compass-editor/src/codemirror/query-autocompleter.ts | Switches query autocompletion to do its own filtering and choose field vs value completion sources. |
| packages/compass-editor/src/codemirror/query-autocompleter.test.ts | Updates tests to validate field/value-aware query completion filtering. |
| packages/compass-editor/src/codemirror/query-autocompleter-with-history.test.ts | Adjusts expectations to reflect the new base query completion filtering behavior. |
| packages/compass-editor/src/codemirror/stage-autocompleter.ts | Adds bson-legacy-uuid to available stage completion meta types. |
| packages/compass-editor/src/codemirror/stage-autocompleter.test.ts | Expands coverage for $match field vs value completion and asserts new meta type. |
| packages/compass-editor/src/codemirror/ace-compat-autocompleter.ts | Removes duplicated completion-mapping logic in favor of shared utility. |
| const infoNode = document.createElement('div'); | ||
| infoNode.classList.add('completion-info'); | ||
| infoNode.addEventListener('mousedown', (evt) => { | ||
| // If we are clicking a link inside the info block, we have to prevent | ||
| // default browser behavior that will remove the focus from the editor | ||
| // and cause the autocompleter to dissapear before browser handles the | ||
| // actual click. This is very similar to how codemirror handles clicks | ||
| // on the list items | ||
| // @see {@link https://github.com/codemirror/autocomplete/blob/82480a7d51d60ad933808e42f6189d841a5a6bc8/src/tooltip.ts#L96-L97} | ||
| if ((evt.target as HTMLElement).nodeName === 'A') { | ||
| evt.preventDefault(); | ||
| } | ||
| }); | ||
| infoNode.innerHTML = completion.description; | ||
| return infoNode; |
There was a problem hiding this comment.
completion.description is inserted via innerHTML, which can lead to XSS if any completion descriptions originate from user-controlled data (e.g., field/schema descriptions). Consider sanitizing allowed markup before insertion (or using textContent for plain-text descriptions and only rendering trusted HTML), and ensure links opened with target="_blank" include rel="noopener noreferrer" to avoid reverse-tabnabbing.
There was a problem hiding this comment.
I was thinking about this one when I moved this function over, we should make the link usage in
more explicit here and restrict the content to text with an optional link that we then create an intentional element for. For the reviewer, think it's worth a ticket? It is a legit XSS possibility if we expanded our autocomplete, and I could see us missing this.
COMPASS-7297
Updates our query autocomplete to do the filtering ourselves instead of passing it all to codemirror. This makes it so we can now offer different results based on if we're in a field or a value in a query.
We still have some indirection with support for ace autocomplete in two of the other autocompleters which I think we can remove altogether. These changes should make that a bit easier as it removed the usage in query. I opted not to refactor unrelated parts as part of these changes.
This did get me thinking, ideally with value autocomplete we'd be suggesting things like
falseortruefor fields we know are booleans, or other types of values that exist in the documents. I reckon it would help folks out. Not doing that here, but something we should keep in mind and discuss when we want to invest more in improving our autocomplete.