Benchmark PR 10

[celmis-codereviewer]

Benchmark reproduction of ai-code-review-evaluation#10

[celmis-codereviewer]

✅ APPROVED — no blocking findings

Full findings and scope are in the review summary comment on this pull request — one persistent comment, updated in place on every run.

[celmis-codereviewer]

❌ CHANGES REQUESTED — blocking findings

Full findings and scope are in the review summary comment on this pull request — one persistent comment, updated in place on every run.

[celmis-codereviewer]

Why: h is interpolated directly into an SQL string without sanitization on line 25, allowing raw SQL execution during database migration.

🔴 SQL injection in database migration

The migration script retrieves host values from the embeddable_hosts site setting and interpolates each element h directly into a raw SQL INSERT statement without escaping or using parameter binding. If the site setting contains single quotes or SQL control characters, it leads to SQL injection during database migration.

Suggested change

	execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES ('#{h}', #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)"
	execute "INSERT INTO embeddable_hosts (host, category_id, created_at, updated_at) VALUES (#{ActiveRecord::Base.sanitize(h)}, #{category_id}, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)"

_{agent: security · rule: sec.cwe-89 · confidence: 0.95}

[celmis-codereviewer]

Why: obj[k] can be null or undefined on line 197; calling .map() on it without a check throws a TypeError when hydrating embedded array attributes.

🟠 Unchecked null array attribute in _hydrateEmbedded causes TypeError

When obj[k] is null or undefined for a key ending in _ids (for example, color_ids: null), obj[k].map(...) is invoked directly without checking if obj[k] is present, causing a JavaScript TypeError: Cannot read property 'map' of null during model hydration.

Suggested change

	const hydrated = obj[k].map(function(id) {
	if (m[2] && Array.isArray(obj[k])) {
	const hydrated = obj[k].map(function(id) {
	return self._lookupSubType(subType, type, id, root);
	});
	obj[self.pluralize(subType)] = hydrated;
	delete obj[k];
	}

_{agent: defect · rule: defect.nil-dereference · confidence: 0.95}

[celmis-codereviewer]

Why: host will be nil on line 10 if params[:id] does not match an existing record; dereferencing host.host on line 23 without a nil check raises a NoMethodError.

🟠 Unchecked nil record in update action causes NoMethodError

When EmbeddableHost.where(id: params[:id]).first finds no record, host is nil. Passing nil to save_host causes host.host = ... on line 23 to raise a NoMethodError: undefined method 'host=' for nil:NilClass instead of gracefully handling the missing record (e.g., using EmbeddableHost.find(params[:id]) or returning a 404).

Also at line 16.

Suggested change

	host = EmbeddableHost.where(id: params[:id]).first
	def update
	host = EmbeddableHost.find(params[:id])
	save_host(host)
	end

_{agent: defect · rule: defect.nil-dereference · confidence: 0.95}

[celmis-codereviewer]

Why: When host is nil on line 6, self.host.sub! raises a NoMethodError because nil does not respond to sub!.

🟠 Calling sub! on nil host in before_validation callback causes NoMethodError

In before_validation, self.host.sub!(...) is called directly without checking if self.host is present. If EmbeddableHost is instantiated or saved with a nil host (such as when creating a record without a host parameter), self.host is nil and sub! raises NoMethodError: undefined method 'sub!' for nil:NilClass instead of failing model validations cleanly.

Suggested change

	self.host.sub!(/^https?:\/\//, '')
	before_validation do
	if self.host.present?
	self.host.sub!(/^https?:\/\//, '')
	self.host.sub!(/\/.*$/, '')
	end
	end

_{agent: defect · rule: defect.nil-dereference · confidence: 0.95}

[celmis-codereviewer]

Why: When no category matches the 'embed_category' site setting on line 11, execute(...) returns an empty result set so [0] evaluates to nil, raising a NoMethodError when dereferencing ['id'].

🟠 Unchecked array indexing on empty query result causes NoMethodError

The query result execute(...) on lines 9–11 will return an empty result set if no site_settings row named 'embed_category' matches a category. Accessing [0] on an empty result set returns nil, and [0]['id'] raises a NoMethodError: undefined method '[]' for nil:NilClass before the if category_id == 0 check on line 14 can execute.

Suggested change

	WHERE s.name = 'embed_category'")[0]['id'].to_i
	result = execute("SELECT c.id FROM categories AS c INNER JOIN site_settings AS s ON s.value = c.name WHERE s.name = 'embed_category'")
	category_id = (result && result[0]) ? result[0]['id'].to_i : 0

_{agent: defect · rule: defect.nil-dereference · confidence: 0.95}

[celmis-codereviewer]

🤖 Code Review for PR #20

⚙ ADJUSTED — graph context partial (32 of 36 changed files): 4 of 36 changed files have no symbols in the index; 3 of them are still in the checkout the index was built from (spec/fabricators/category_fabricator.rb, spec/models/site_setting_spec.rb, spec/models/topic_embed_spec.rb) — the index is stale there, or the extractor could not parse it; run analyzer generate or index it from the Repositories page (POST /api/repos/index-all); 1 of them is not in that checkout at all (spec/controllers/embed_controller_spec.rb) — this PR's base is older than the indexed revision, so those files were renamed or deleted before it and no re-index can bring them back; there is nothing to fix.

❌ CHANGES REQUESTED — blocking findings

Findings

• 🔴 Critical: 1
• 🟠 Error: 4

Scope

• Files changed: 36
• Lines: +449 / -127

Performance

• Analysis time: 247.5s · agents: structural, cve, contract, security, defect · tokens: 52,587/30,735

---

_{Powered by Code Analyzer · context: tree-sitter graph + structural, cve, contract, security, defect}