FEATURE: Can edit category/host relationships for embedding #1
Conversation
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| records = val.split("\n") | ||
| if records.present? | ||
| records.each do |h| | ||
| execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" |
There was a problem hiding this comment.
Bug: Unsafe Input Leads to SQL Injection
SQL injection vulnerability in migration: the variable h from user-controlled site settings is directly interpolated into the SQL query without escaping. If embeddable_hosts setting contains malicious input with single quotes, it could execute arbitrary SQL. Use parameterized queries or proper escaping.
|
|
||
| category_id = execute("SELECT c.id FROM categories AS c | ||
| INNER JOIN site_settings AS s ON s.value = c.name | ||
| WHERE s.name = 'embed_category'")[0]['id'].to_i |
There was a problem hiding this comment.
Bug: Migration Failure: Unsafe Data Access
Migration assumes the query for embed_category will return at least one result and accesses [0]['id'] without checking. If the site setting doesn't exist or the category name doesn't match, this will raise a NoMethodError when calling ['id'] on nil, causing the migration to fail.
|
|
||
| def update | ||
| host = EmbeddableHost.where(id: params[:id]).first | ||
| save_host(host) |
There was a problem hiding this comment.
Bug: Handle Missing Records Gracefully
The update action calls save_host(host) where host could be nil if no record with the given id exists. This will cause NoMethodError when save_host tries to set attributes on nil. The method should check if the host exists before proceeding.
|
|
||
| def destroy | ||
| host = EmbeddableHost.where(id: params[:id]).first | ||
| host.destroy |
There was a problem hiding this comment.
Bug: Destroy Fails Gracefully on Missing Resource
The destroy action calls host.destroy where host could be nil if no record with the given id exists. This will raise NoMethodError: undefined method 'destroy' for nil:NilClass instead of returning a proper 404 response.
Test 10
Replicated from ai-code-review-evaluation/discourse-cursor#10
Note
Adds a DB-backed
EmbeddableHostwith admin UI to manage host→category mappings, replaces old site settings, and updates embedding logic and client hydration.embeddable_hostsunderCustomize → Embedding.EmbeddableHostmodel, serializers, and CRUD (Admin::EmbeddableHostsController).Admin::EmbeddingControllerto expose embedding config.EmbeddableHost.host_allowed?inEmbedControllerandTopicRetriever.TopicEmbed.importassigns category viaEmbeddableHost.record_for_host.Topic#expandable_first_post?no longer depends on embeddable hosts setting./admin/customize/embeddingandembeddable_hostsresources.embeddable_hostsand import from removed settings.embeddable-hostas admin model; basePath fix for dashed types._idsarrays (e.g.,color_ids→colors).embedding.embeddable_hosts/embed_categorysettings.EmbeddableHostand embedding._idshydration.Written by Cursor Bugbot for commit d1c6918. Configure here.