feat(agents): analytics + GEO tools for eve and Iris - #649
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Comp AI code review complete — no issues found. Commit |
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
React Doctor found 30 new issues in 17 files · 3 errors & 27 warnings · score 70 / 100 (Needs work) · 17 fixed · vs Errors
27 warnings
Reviewed by React Doctor for commit |
Greptile SummaryThe PR adds seven analytics/GEO tools to Eve and extends Iris with analytics-aware planning and A/B-test capabilities, together with planner and GEO evaluation harnesses.
Confidence Score: 4/5The A/B-test workflow needs to expose platform post IDs before merging, otherwise Eve and Iris cannot create experiments grounded in the analytics results they are instructed to use. Both agent paths discard Files Needing Attention: packages/tools/src/analytics/get-top-posts.ts, packages/ai/src/autonomy/capabilities.ts, packages/ai/src/prompts/iris-planner.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent as Eve / Iris
participant Analytics as Analytics read
participant Tinybird
participant Experiment as Experiment create
participant DB as socialExperiments
Agent->>Analytics: Request top posts
Analytics->>Tinybird: queryTopPosts(organization_id)
Tinybird-->>Analytics: Posts including platform_post_id
Analytics-->>Agent: Posts without platform_post_id
Agent->>Experiment: Create test requiring two post IDs
Experiment->>DB: Persist supplied IDs
Note over Agent,DB: Valid IDs cannot be sourced from the advertised read
Reviews (1): Last reviewed commit: "feat(agents): add analytics and GEO tool..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| content: row.content, | ||
| url: row.url, | ||
| posted_at: row.posted_at, | ||
| likes: row.likes, | ||
| replies: row.replies, | ||
| reposts: row.reposts, | ||
| impressions: row.impressions, | ||
| engagement: row.engagement, | ||
| })), | ||
| }; |
There was a problem hiding this comment.
When Eve or Iris creates an A/B test from the new analytics reads, both result mappings discard the available platform_post_id even though experiment creation requires those IDs, forcing the agents to omit the experiment or submit invented IDs that persist with empty or zero-valued metrics.
Knowledge Base Used:
![comp-ai-code-review[bot]](https://avatars.githubusercontent.com/in/2105518?s=60&v=4){kind=small}
There was a problem hiding this comment.
No blocking issues found across the changed files.
1 other commit review still in progress for this PR — findings may follow.
Commit 9514ad2 · Posted by Comp AI Code Reviews.
There was a problem hiding this comment.
8 issues found across 8 files.
Prompt for AI agents (all issues)
Check whether each issue below is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it, and use sub-agents to handle independent issues in parallel.
<file name="apps/agent/agent/tools/get_social_analytics_overview.ts">
<issue n="1" at="apps/agent/agent/tools/get_social_analytics_overview.ts:46-50" severity="MEDIUM">Raw backend error messages surfaced to tool caller via catch block — This target file is a thin wrapper (`export default createGetSocialAnalyticsOverviewTool()`) that delegates to the implementation in `packages/tools/src/analytics/get-social-analytics-overview.ts`. That implementation's `execute` catch block (lines 46-50) returns `${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}` directly as the tool result. In an AI-agent context, tool return values are fed to the LLM and frequently relayed verbatim to end users. Because `querySocialOverview` calls the Tinybird SDK, a failing request can propagate third-party backend error text — e.g. Tinybird endpoint/pipe names, SQL fragments, rate-limit headers, or 'Unauthorized / invalid token' hints that reveal the analytics backend configuration and confirm whether `TINYBIRD_TOKEN` is set. The query is scoped to the caller's own `organization_id`, so cross-organization data is not leaked, but internal backend/integration details are. Positive note: the tool otherwise correctly sources `organizationId` from `ctx.session.auth` (not user tool input, since the input schema is an empty object), uses Tinybird's parameterized `{{String(organization_id)}}` template (no SQL injection), and validates all inputs — so this error-handling hygiene issue is the only finding. Fix: In production, return only the generic `ANALYTICS_QUERY_FAILED_MESSAGE` to the tool caller and log the full `error` (including stack) server-side via the agent's structured logger. If diagnostics are needed, gate detailed error text behind a non-production environment flag so internal Tinybird backend details are never surfaced to end users through the agent.</issue>
</file>
<file name="apps/agent/agent/tools/get_top_posts.ts">
<issue n="2" at="apps/agent/agent/tools/get_top_posts.ts:48-52" severity="MEDIUM">Raw backend error messages surfaced to tool caller via catch block — This target file is a thin wrapper (`export default createGetTopPostsTool()`) that delegates to the implementation in `packages/tools/src/analytics/get-top-posts.ts`. That implementation's `execute` catch block (lines 48-52) returns `${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}` directly as the tool result. In an AI-agent context, tool return values are fed to the LLM and often relayed verbatim to end users. Since `queryTopPosts` invokes the Tinybird SDK, a failing request can expose third-party backend error text — Tinybird pipe/endpoint names, SQL fragments, rate-limit info, or token-validity hints that disclose the analytics backend configuration. The query is scoped to the caller's own `organization_id` (no cross-org leak), and `limit` is zod-validated to int 1-25 with default 10 and passed to Tinybird's parameterized `{{Int32(limit, 10)}}` template (no SQL injection). `organizationId` is correctly derived from `ctx.session.auth`, not user tool input. The raw-error disclosure is the only finding. Fix: In production, return only the generic `ANALYTICS_QUERY_FAILED_MESSAGE` to the tool caller and log the full `error` (including stack) server-side via the agent's structured logger. Gate any detailed error text behind a non-production environment flag so internal Tinybird backend details are never surfaced to end users through the agent.</issue>
</file>
<file name="packages/tools/src/analytics/get-social-analytics-overview.ts">
<issue n="3" at="packages/tools/src/analytics/get-social-analytics-overview.ts:31-34" severity="MEDIUM">Raw backend error messages returned to the agent context — The catch block (L31-34) appends the raw exception text to ANALYTICS_QUERY_FAILED_MESSAGE and returns it: `${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}`. When the underlying Tinybird client throws, the error message can include internal backend details (request URLs, datasource names, partial SQL context, token presence indicators, or stack traces from the SDK). This string is returned from the tool's execute() into the agent/LLM context and may be surfaced back to the end user via the agent's reply. The scanner's 'insecure-crypto / weak cipher algorithm' flag at L15 is a false positive — there is no cryptographic code in this file; line 15 is the `inputSchema` assignment. The Tinybird queries themselves are safe: the endpoint definitions in packages/analytics/src/tinybird/endpoints.ts use parameterized Tinybird templates (`{{String(organization_id)}}`) rather than string interpolation, so there is no SQL injection. The organizationId is sourced from the trusted session context (ctx.session.auth.current?.attributes.organizationId), which is populated by the authenticated caller (Vercel OIDC project verification or HTTP basic service account in apps/agent/agent/channels/eve.ts) rather than directly from end-user input. Fix: Return a generic, fixed error message to the agent context and log the detailed error server-side only. Do not interpolate `error.message` / `String(error)` into tool output, since the agent may relay it to end users.</issue>
</file>
<file name="packages/tools/src/analytics/get-top-posts.ts">
<issue n="4" at="packages/tools/src/analytics/get-top-posts.ts:36-39" severity="MEDIUM">Raw backend error messages returned to the agent context — The catch block (L36-39) appends the raw exception text to ANALYTICS_QUERY_FAILED_MESSAGE and returns it: `${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}`. When the underlying Tinybird client throws, the error message can include internal backend details (request URLs, datasource names, partial SQL context, token presence indicators, or stack traces from the SDK). This string is returned from the tool's execute() into the agent/LLM context and may be surfaced back to the end user via the agent's reply. The scanner's 'insecure-crypto / weak cipher algorithm' flag at L15 is a false positive — there is no cryptographic code in this file; line 15 is `return defineTool({`. The Tinybird queries themselves are safe: the endpoint definitions in packages/analytics/src/tinybird/endpoints.ts use parameterized Tinybird templates (`{{String(organization_id)}}`, `{{Int32(limit, 10)}}`) rather than string interpolation, so there is no SQL injection, and `limit` is additionally constrained by the Zod schema (z.number().int().min(1).max(25).default(10)). The organizationId is sourced from the trusted session context (ctx.session.auth.current?.attributes.organizationId), which is populated by the authenticated caller (Vercel OIDC project verification or HTTP basic service account in apps/agent/agent/channels/eve.ts) rather than directly from end-user input. Fix: Return a generic, fixed error message to the agent context and log the detailed error server-side only. Do not interpolate `error.message` / `String(error)` into tool output, since the agent may relay it to end users.</issue>
</file>
<file name="apps/agent/agent/tools/create_ab_test.ts">
<issue n="5" at="apps/agent/agent/tools/create_ab_test.ts:22-33" severity="MEDIUM">create_ab_test accepts arbitrary post IDs without ownership validation or length bounds — The create_ab_test tool (implementation in packages/tools/src/analytics/create-ab-test.ts, re-exported by this file) inserts an A/B experiment row using `variantAPostId` and `variantBPostId` taken directly from LLM/user input. The input schema (`createAbTestInputSchema`) only constrains these as `z.string().min(1)` with no maximum length and no validation that the post IDs belong to the calling organization. The only check is that A != B. The row is then written to the `social_experiments` table scoped to the session's `organizationId` (which IS trustworthy — derived from the authenticated dashboard/API caller, not user input). Trust model analysis: the `organizationId` comes from `requireOrganizationId(ctx)`, which reads `ctx.session.auth...attributes.organizationId`, set from the `x-notra-organization-id` header by trusted, authenticated callers (dashboard/API derive it from authenticated API keys/sessions, not user input), so this is NOT a cross-tenant access bypass. However, because post IDs are never checked against the org's actual posts (e.g. via Tinybird `post_metrics_lookup` or the social_posts table), a caller can create experiment rows referencing arbitrary/garbage or extremely long strings. Impact is limited to data pollution within the caller's own organization: `get_ab_tests` resolves metrics through Tinybird's `post_metrics_lookup` pipe which is itself scoped by `organization_id`, so a cross-organization post ID simply yields null metrics rather than leaking another tenant's data. The DB column is `text`, so no crash, but there is no length cap to constrain storage abuse. This is a data-integrity/validation weakness rather than an exploitable security vulnerability. Fix: Validate that variantAPostId and variantBPostId correspond to real posts owned by the calling organization before inserting (e.g. run them through Tinybird's post_metrics_lookup scoped by organization_id, or query social_posts WHERE organization_id = ? AND platform_post_id IN (...), and reject if any are missing). Also add a reasonable max length (e.g. .max(100)) to the post ID strings to prevent storage abuse, consistent with platform post id formats.</issue>
</file>
<file name="packages/ai/src/prompts/iris-planner.ts">
<issue n="6" at="packages/ai/src/prompts/iris-planner.ts:206-211" severity="MEDIUM">recentActionSummaries interpolated into planner prompt without sanitizeUntrustedText (inconsistent with signalSummaries) — In buildIrisPlannerUserPrompt, signalSummaries are passed through sanitizeUntrustedText and placed inside the SIGNAL_DELIMITER_OPEN/CLOSE block (L211), and mandate.objective is also sanitized (L188). However, recentActionSummaries are interpolated raw into the <recent-actions> block via describeList(input.recentActionSummaries, ...) (L206) with neither sanitizeUntrustedText nor delimiter wrapping. The system prompt only instructs the model to distrust text inside the signal delimiters, so content in <recent-actions> is implicitly treated as trusted. I traced the data flow: loadRecentActionSummaries (apps/dashboard/src/lib/iris/history.ts) builds each summary from a fixed template `${capabilityName} ${status} at ${createdAt.toISOString()}` using capabilityName (validated against the capability catalog enum), status (an enum), and an ISO date. Because these are constrained enum-style DB columns rather than attacker-controlled free text, this is NOT currently exploitable — hence BUG rather than a security severity. The concern is latent: if loadRecentActionSummaries is later extended to include free-text fields (e.g., an action's goal title or reason, which the planner derives from untrusted signal/commit data), the unsanitized interpolation would silently become an indirect prompt-injection path that bypasses the delimiter-trust boundary the rest of the file enforces. The scanner's 'insecure-crypto' flags on this file are false positives — there is no cryptographic code anywhere in the file (confirmed by grep for crypto/hash/AES/jwt/Math.random). Fix: Apply sanitizeUntrustedText consistently: change L206 to describeList(input.recentActionSummaries.map(sanitizeUntrustedText), "- none recorded yet") to match the treatment of signalSummaries, so the trust boundary holds even if the summary format is later changed to include free text.</issue>
</file>
<file name="packages/tools/src/analytics/create-ab-test.ts">
<issue n="7" at="packages/tools/src/analytics/create-ab-test.ts:11-39" severity="MEDIUM">A/B test variant post IDs are not validated for ownership/existence before insert — createCreateAbTestTool accepts `variantAPostId` and `variantBPostId` as free-form `z.string().min(1)` values (see schemas/analytics-tools.ts) and inserts them directly into the `social_experiments` table without verifying that those post IDs belong to the caller's organization or even exist. The only check is that the two IDs differ. The experiment row itself is correctly scoped to the caller's `organizationId` (from the authenticated session via requireOrganizationId), and when metrics are later looked up in get-ab-tests.ts the Tinybird `post_metrics_lookup` pipe is called with `organization_id: organizationId` plus the post_ids, so a post ID belonging to another org would return no metrics (no cross-tenant data exfiltration). The practical impact is therefore limited to data-quality/logic bugs: a user (or a confused agent) can create experiments referencing arbitrary or non-existent post IDs, producing A/B tests that silently never resolve. This is not a security boundary violation because no other tenant's data is ever returned, but it is a missing ownership check worth noting. Note: the scanner's 'insecure-crypto' / 'weak cipher algorithm' flags on the import line and 'crypto-usage' on `crypto.randomUUID()` are false positives — `crypto.randomUUID()` is the CSPRNG-backed Web Crypto API and is the correct way to generate record IDs; no weak cipher is used anywhere in this file. Fix: Before inserting, validate that variantAPostId/variantBPostId correspond to real posts owned by the caller's organization (e.g., query the org-scoped posts/social_posts table or Tinybird datasource and reject unknown IDs). Optionally add a DB-level foreign key to an org-scoped posts table if such a table exists.</issue>
</file>
<file name="packages/tools/src/schemas/analytics-tools.ts">
<issue n="8" at="packages/tools/src/schemas/analytics-tools.ts:52-57" severity="MEDIUM">createAbTestInputSchema accepts variant post IDs without ownership validation — The createAbTestInputSchema defines variantAPostId and variantBPostId as free-form strings (z.string().min(1)) described as 'Platform post id of variant A/B (from get_top_posts)'. In the consuming tool (packages/tools/src/analytics/create-ab-test.ts), these user/LLM-supplied post IDs are inserted directly into the socialExperiments table without any verification that the referenced posts belong to the calling organization. The socialExperiments table schema (packages/db/src/schema.ts L1501-1510) stores variantAPostId/variantBPostId as plain text columns with no foreign key or organization-scoped constraint. While the actual security impact is limited — the read path in get-ab-tests.ts queries Tinybird scoped by organization_id, so metrics for a foreign post ID would return no data rather than leaking another tenant's metrics — this still represents a data integrity gap. A user could create experiments referencing arbitrary or garbage post IDs, polluting the experiments table. The scanner's 'weak cipher algorithm' hits on lines 12, 22, 32, 42, 52, 57, 61, 65, 69, 73 are all false positives: this file contains only Zod schema definitions with .default() values and enums, and no cryptographic code whatsoever. The actual encryption implementation in packages/db/src/utils/integration-encryption.ts uses proper AES-256-GCM with random IVs and auth tags. Fix: Before inserting a new A/B test, validate that variantAPostId and variantBPostId correspond to posts that belong to the calling organization. Query Tinybird's top posts endpoint (or the socialPosts data source) scoped by organization_id and verify both post IDs exist in the result set before allowing the insert. This prevents users from creating experiments referencing arbitrary post IDs.</issue>
</file>
Commit d32a58e · Posted by Comp AI Code Reviews.
| return ANALYTICS_NOT_CONFIGURED_MESSAGE; | ||
| } | ||
|
|
||
| return { |
There was a problem hiding this comment.
MEDIUM: Raw backend error messages returned to the agent context
The catch block (L31-34) appends the raw exception text to ANALYTICS_QUERY_FAILED_MESSAGE and returns it: ${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}. When the underlying Tinybird client throws, the error message can include internal backend details (request URLs, datasource names, partial SQL context, token presence indicators, or stack traces from the SDK). This string is returned from the tool's execute() into the agent/LLM context and may be surfaced back to the end user via the agent's reply. The scanner's 'insecure-crypto / weak cipher algorithm' flag at L15 is a false positive — there is no cryptographic code in this file; line 15 is the inputSchema assignment. The Tinybird queries themselves are safe: the endpoint definitions in packages/analytics/src/tinybird/endpoints.ts use parameterized Tinybird templates ({{String(organization_id)}}) rather than string interpolation, so there is no SQL injection. The organizationId is sourced from the trusted session context (ctx.session.auth.current?.attributes.organizationId), which is populated by the authenticated caller (Vercel OIDC project verification or HTTP basic service account in apps/agent/agent/channels/eve.ts) rather than directly from end-user input.
Suggestion: Return a generic, fixed error message to the agent context and log the detailed error server-side only. Do not interpolate error.message / String(error) into tool output, since the agent may relay it to end users.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/tools/src/analytics/get-social-analytics-overview.ts:31-34" severity="MEDIUM">Raw backend error messages returned to the agent context — The catch block (L31-34) appends the raw exception text to ANALYTICS_QUERY_FAILED_MESSAGE and returns it: `${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}`. When the underlying Tinybird client throws, the error message can include internal backend details (request URLs, datasource names, partial SQL context, token presence indicators, or stack traces from the SDK). This string is returned from the tool's execute() into the agent/LLM context and may be surfaced back to the end user via the agent's reply. The scanner's 'insecure-crypto / weak cipher algorithm' flag at L15 is a false positive — there is no cryptographic code in this file; line 15 is the `inputSchema` assignment. The Tinybird queries themselves are safe: the endpoint definitions in packages/analytics/src/tinybird/endpoints.ts use parameterized Tinybird templates (`{{String(organization_id)}}`) rather than string interpolation, so there is no SQL injection. The organizationId is sourced from the trusted session context (ctx.session.auth.current?.attributes.organizationId), which is populated by the authenticated caller (Vercel OIDC project verification or HTTP basic service account in apps/agent/agent/channels/eve.ts) rather than directly from end-user input. Fix: Return a generic, fixed error message to the agent context and log the detailed error server-side only. Do not interpolate `error.message` / `String(error)` into tool output, since the agent may relay it to end users.</issue>
Commit d32a58e.
| posts: result.data.map((row) => ({ | ||
| provider: row.provider, | ||
| content: row.content, | ||
| url: row.url, |
There was a problem hiding this comment.
MEDIUM: Raw backend error messages returned to the agent context
The catch block (L36-39) appends the raw exception text to ANALYTICS_QUERY_FAILED_MESSAGE and returns it: ${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}. When the underlying Tinybird client throws, the error message can include internal backend details (request URLs, datasource names, partial SQL context, token presence indicators, or stack traces from the SDK). This string is returned from the tool's execute() into the agent/LLM context and may be surfaced back to the end user via the agent's reply. The scanner's 'insecure-crypto / weak cipher algorithm' flag at L15 is a false positive — there is no cryptographic code in this file; line 15 is return defineTool({. The Tinybird queries themselves are safe: the endpoint definitions in packages/analytics/src/tinybird/endpoints.ts use parameterized Tinybird templates ({{String(organization_id)}}, {{Int32(limit, 10)}}) rather than string interpolation, so there is no SQL injection, and limit is additionally constrained by the Zod schema (z.number().int().min(1).max(25).default(10)). The organizationId is sourced from the trusted session context (ctx.session.auth.current?.attributes.organizationId), which is populated by the authenticated caller (Vercel OIDC project verification or HTTP basic service account in apps/agent/agent/channels/eve.ts) rather than directly from end-user input.
Suggestion: Return a generic, fixed error message to the agent context and log the detailed error server-side only. Do not interpolate error.message / String(error) into tool output, since the agent may relay it to end users.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/tools/src/analytics/get-top-posts.ts:36-39" severity="MEDIUM">Raw backend error messages returned to the agent context — The catch block (L36-39) appends the raw exception text to ANALYTICS_QUERY_FAILED_MESSAGE and returns it: `${ANALYTICS_QUERY_FAILED_MESSAGE} ${error instanceof Error ? error.message : String(error)}`. When the underlying Tinybird client throws, the error message can include internal backend details (request URLs, datasource names, partial SQL context, token presence indicators, or stack traces from the SDK). This string is returned from the tool's execute() into the agent/LLM context and may be surfaced back to the end user via the agent's reply. The scanner's 'insecure-crypto / weak cipher algorithm' flag at L15 is a false positive — there is no cryptographic code in this file; line 15 is `return defineTool({`. The Tinybird queries themselves are safe: the endpoint definitions in packages/analytics/src/tinybird/endpoints.ts use parameterized Tinybird templates (`{{String(organization_id)}}`, `{{Int32(limit, 10)}}`) rather than string interpolation, so there is no SQL injection, and `limit` is additionally constrained by the Zod schema (z.number().int().min(1).max(25).default(10)). The organizationId is sourced from the trusted session context (ctx.session.auth.current?.attributes.organizationId), which is populated by the authenticated caller (Vercel OIDC project verification or HTTP basic service account in apps/agent/agent/channels/eve.ts) rather than directly from end-user input. Fix: Return a generic, fixed error message to the agent context and log the detailed error server-side only. Do not interpolate `error.message` / `String(error)` into tool output, since the agent may relay it to end users.</issue>
Commit d32a58e.
| "Start a social A/B test comparing two published posts on one metric. Use post ids from get_top_posts (the platform_post_id field). The test tracks both posts' live metrics until a winner is declared in the dashboard. Use this to run data-driven experiments on hooks, formats, or topics.", | ||
| inputSchema: createAbTestInputSchema, | ||
| async execute(input, ctx) { | ||
| const organizationId = requireOrganizationId(ctx); | ||
|
|
||
| if (input.variantAPostId === input.variantBPostId) { | ||
| return "Variant A and variant B must be different posts."; | ||
| } | ||
|
|
||
| try { | ||
| const [created] = await db | ||
| .insert(socialExperiments) | ||
| .values({ | ||
| id: crypto.randomUUID(), | ||
| organizationId, | ||
| name: input.name, | ||
| hypothesis: input.hypothesis ?? null, | ||
| provider: input.provider, | ||
| variantAPostId: input.variantAPostId, | ||
| variantBPostId: input.variantBPostId, | ||
| metric: input.metric, | ||
| }) | ||
| .returning({ id: socialExperiments.id }); | ||
|
|
||
| return { | ||
| experiment_id: created?.id ?? null, | ||
| status: "running", | ||
| note: "Metrics update on every analytics sync. Check results with get_ab_tests.", | ||
| }; |
There was a problem hiding this comment.
MEDIUM: A/B test variant post IDs are not validated for ownership/existence before insert
createCreateAbTestTool accepts variantAPostId and variantBPostId as free-form z.string().min(1) values (see schemas/analytics-tools.ts) and inserts them directly into the social_experiments table without verifying that those post IDs belong to the caller's organization or even exist. The only check is that the two IDs differ. The experiment row itself is correctly scoped to the caller's organizationId (from the authenticated session via requireOrganizationId), and when metrics are later looked up in get-ab-tests.ts the Tinybird post_metrics_lookup pipe is called with organization_id: organizationId plus the post_ids, so a post ID belonging to another org would return no metrics (no cross-tenant data exfiltration). The practical impact is therefore limited to data-quality/logic bugs: a user (or a confused agent) can create experiments referencing arbitrary or non-existent post IDs, producing A/B tests that silently never resolve. This is not a security boundary violation because no other tenant's data is ever returned, but it is a missing ownership check worth noting. Note: the scanner's 'insecure-crypto' / 'weak cipher algorithm' flags on the import line and 'crypto-usage' on crypto.randomUUID() are false positives — crypto.randomUUID() is the CSPRNG-backed Web Crypto API and is the correct way to generate record IDs; no weak cipher is used anywhere in this file.
Suggestion: Before inserting, validate that variantAPostId/variantBPostId correspond to real posts owned by the caller's organization (e.g., query the org-scoped posts/social_posts table or Tinybird datasource and reject unknown IDs). Optionally add a DB-level foreign key to an org-scoped posts table if such a table exists.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/tools/src/analytics/create-ab-test.ts:11-39" severity="MEDIUM">A/B test variant post IDs are not validated for ownership/existence before insert — createCreateAbTestTool accepts `variantAPostId` and `variantBPostId` as free-form `z.string().min(1)` values (see schemas/analytics-tools.ts) and inserts them directly into the `social_experiments` table without verifying that those post IDs belong to the caller's organization or even exist. The only check is that the two IDs differ. The experiment row itself is correctly scoped to the caller's `organizationId` (from the authenticated session via requireOrganizationId), and when metrics are later looked up in get-ab-tests.ts the Tinybird `post_metrics_lookup` pipe is called with `organization_id: organizationId` plus the post_ids, so a post ID belonging to another org would return no metrics (no cross-tenant data exfiltration). The practical impact is therefore limited to data-quality/logic bugs: a user (or a confused agent) can create experiments referencing arbitrary or non-existent post IDs, producing A/B tests that silently never resolve. This is not a security boundary violation because no other tenant's data is ever returned, but it is a missing ownership check worth noting. Note: the scanner's 'insecure-crypto' / 'weak cipher algorithm' flags on the import line and 'crypto-usage' on `crypto.randomUUID()` are false positives — `crypto.randomUUID()` is the CSPRNG-backed Web Crypto API and is the correct way to generate record IDs; no weak cipher is used anywhere in this file. Fix: Before inserting, validate that variantAPostId/variantBPostId correspond to real posts owned by the caller's organization (e.g., query the org-scoped posts/social_posts table or Tinybird datasource and reject unknown IDs). Optionally add a DB-level foreign key to an org-scoped posts table if such a table exists.</issue>
Commit d32a58e.
| .describe("Short descriptive name for the experiment."), | ||
| hypothesis: z | ||
| .string() | ||
| .max(500) | ||
| .optional() | ||
| .describe("What you expect to learn and why."), |
There was a problem hiding this comment.
MEDIUM: createAbTestInputSchema accepts variant post IDs without ownership validation
The createAbTestInputSchema defines variantAPostId and variantBPostId as free-form strings (z.string().min(1)) described as 'Platform post id of variant A/B (from get_top_posts)'. In the consuming tool (packages/tools/src/analytics/create-ab-test.ts), these user/LLM-supplied post IDs are inserted directly into the socialExperiments table without any verification that the referenced posts belong to the calling organization. The socialExperiments table schema (packages/db/src/schema.ts L1501-1510) stores variantAPostId/variantBPostId as plain text columns with no foreign key or organization-scoped constraint. While the actual security impact is limited — the read path in get-ab-tests.ts queries Tinybird scoped by organization_id, so metrics for a foreign post ID would return no data rather than leaking another tenant's metrics — this still represents a data integrity gap. A user could create experiments referencing arbitrary or garbage post IDs, polluting the experiments table. The scanner's 'weak cipher algorithm' hits on lines 12, 22, 32, 42, 52, 57, 61, 65, 69, 73 are all false positives: this file contains only Zod schema definitions with .default() values and enums, and no cryptographic code whatsoever. The actual encryption implementation in packages/db/src/utils/integration-encryption.ts uses proper AES-256-GCM with random IVs and auth tags.
Suggestion: Before inserting a new A/B test, validate that variantAPostId and variantBPostId correspond to posts that belong to the calling organization. Query Tinybird's top posts endpoint (or the socialPosts data source) scoped by organization_id and verify both post IDs exist in the result set before allowing the insert. This prevents users from creating experiments referencing arbitrary post IDs.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/tools/src/schemas/analytics-tools.ts:52-57" severity="MEDIUM">createAbTestInputSchema accepts variant post IDs without ownership validation — The createAbTestInputSchema defines variantAPostId and variantBPostId as free-form strings (z.string().min(1)) described as 'Platform post id of variant A/B (from get_top_posts)'. In the consuming tool (packages/tools/src/analytics/create-ab-test.ts), these user/LLM-supplied post IDs are inserted directly into the socialExperiments table without any verification that the referenced posts belong to the calling organization. The socialExperiments table schema (packages/db/src/schema.ts L1501-1510) stores variantAPostId/variantBPostId as plain text columns with no foreign key or organization-scoped constraint. While the actual security impact is limited — the read path in get-ab-tests.ts queries Tinybird scoped by organization_id, so metrics for a foreign post ID would return no data rather than leaking another tenant's metrics — this still represents a data integrity gap. A user could create experiments referencing arbitrary or garbage post IDs, polluting the experiments table. The scanner's 'weak cipher algorithm' hits on lines 12, 22, 32, 42, 52, 57, 61, 65, 69, 73 are all false positives: this file contains only Zod schema definitions with .default() values and enums, and no cryptographic code whatsoever. The actual encryption implementation in packages/db/src/utils/integration-encryption.ts uses proper AES-256-GCM with random IVs and auth tags. Fix: Before inserting a new A/B test, validate that variantAPostId and variantBPostId correspond to posts that belong to the calling organization. Query Tinybird's top posts endpoint (or the socialPosts data source) scoped by organization_id and verify both post IDs exist in the result set before allowing the insert. This prevents users from creating experiments referencing arbitrary post IDs.</issue>
Commit d32a58e.
There was a problem hiding this comment.
No blocking issues found across the changed files.
Commit 14d90d9 · Posted by Comp AI Code Reviews.
There was a problem hiding this comment.
No blocking issues found across the changed files.
Commit a015938 · Posted by Comp AI Code Reviews.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const [stage, setStage] = useState(0); | ||
| const ready = !isOverviewPending; | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
React Doctor · react-doctor/effect-needs-cleanup (error)
setTimeout creates a timer in useEffect without guaranteed cleanup. Return a cleanup function that owns every allocation so it does not leak after unmount.
Fix → Return a cleanup function that stops the subscription or timer: return () => target.removeEventListener(name, handler) for listeners, return () => clearInterval(id) or clearTimeout(id) for timers, return () => observer.disconnect() for observers, return () => socket.close() for connections, or return unsubscribe if the subscribe call already gave you one.
|
|
||
| useEffect(() => { | ||
| if (!ready) { | ||
| setStage(0); |
There was a problem hiding this comment.
React Doctor · react-hooks-js/set-state-in-effect (warning)
This synchronous effect update causes an extra render: Calling setState synchronously within an effect can trigger cascading renders. Prefer deriving or initializing the value before render. If the effect must read a browser API after mount, treat this as advisory or suppress it with // react-doctor-disable-next-line react-hooks-js/set-state-in-effect.
Fix → Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
- Update external systems with the latest state from React.
- Subscribe for updates from some external system, calling setState in a callback function when external state changes.
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).
| const [stage, setStage] = useState(0); | ||
| const ready = !isSettingsPending; | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
React Doctor · react-doctor/effect-needs-cleanup (error)
setTimeout creates a timer in useEffect without guaranteed cleanup. Return a cleanup function that owns every allocation so it does not leak after unmount.
Fix → Return a cleanup function that stops the subscription or timer: return () => target.removeEventListener(name, handler) for listeners, return () => clearInterval(id) or clearTimeout(id) for timers, return () => observer.disconnect() for observers, return () => socket.close() for connections, or return unsubscribe if the subscribe call already gave you one.
|
|
||
| useEffect(() => { | ||
| if (!ready) { | ||
| setStage(0); |
There was a problem hiding this comment.
React Doctor · react-hooks-js/set-state-in-effect (warning)
This synchronous effect update causes an extra render: Calling setState synchronously within an effect can trigger cascading renders. Prefer deriving or initializing the value before render. If the effect must read a browser API after mount, treat this as advisory or suppress it with // react-doctor-disable-next-line react-hooks-js/set-state-in-effect.
Fix → Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
- Update external systems with the latest state from React.
- Subscribe for updates from some external system, calling setState in a callback function when external state changes.
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).
| <ChartSeriesLegend | ||
| <InstrumentModule eyebrow={title} readout={readout}> | ||
| {hasData ? ( | ||
| <Chart |
There was a problem hiding this comment.
React Doctor · react-hooks-js/static-components (error)
This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot create components during render. Rewrite the flagged code so the compiler can optimize it.
Fix → Components created during render will reset their state each time they are created. Declare components outside of render.
| engagement: Number(row.engagement), | ||
| }; | ||
| }), | ||
| posts: (result?.data ?? []) |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .filter().map() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
| ? Math.round((groundedRate - rawRate) * PERCENT) | ||
| : null; | ||
| const bestEngine = | ||
| [...engines].sort((a, b) => b.mentionRate - a.mentionRate)[0] ?? null; |
There was a problem hiding this comment.
React Doctor · react-doctor/js-tosorted-immutable (warning)
This wastes work because [...array].sort() copies the array just to sort it, so use array.toSorted() to sort without the extra copy (ES2023)
Fix → Use array.toSorted() (ES2023) instead of [...array].sort() so you sort without copying the array first
| } | ||
| try { | ||
| const files = await readdir(RESULTS_DIR); | ||
| const rounds = files |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .map().filter() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
| const results: ScenarioResult[] = []; | ||
| for (const scenario of IRIS_EVAL_SCENARIOS) { | ||
| process.stdout.write(` ${scenario.id} ...\n`); | ||
| results.push(await runScenario(scenario)); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This makes the for…of loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))
Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time
| engagement: row.engagement, | ||
| })); | ||
|
|
||
| const bestWeekdays = [...performance.data] |
There was a problem hiding this comment.
React Doctor · react-doctor/js-tosorted-immutable (warning)
This wastes work because [...array].sort() copies the array just to sort it, so use array.toSorted() to sort without the extra copy (ES2023)
Fix → Use array.toSorted() (ES2023) instead of [...array].sort() so you sort without copying the array first
There was a problem hiding this comment.
No blocking issues found across the changed files.
Commit d9751ba · Posted by Comp AI Code Reviews.
There was a problem hiding this comment.
No blocking issues found across the changed files.
Commit df3d0d7 · Posted by Comp AI Code Reviews.
Remove the track button and dialog, drop the rank-change column, and pin the window select to the standard module header height so row lines align across modules.
There was a problem hiding this comment.
No blocking issues found across the changed files.
Commit 50839b4 · Posted by Comp AI Code Reviews.
1 participant
Exposes everything in the stack below to the agents, so eve and Iris can read analytics and act on it.
@notra/toolsanalytics tools: social analytics overview, engagement timeseries, top posts, posting performance, GEO overview, list A/B tests and create an A/B test.apps/agent.Stack
Summary by cubic
Adds analytics and GEO tools so Eve and Iris can read social metrics, run A/B tests, act on GEO visibility, detect AI‑agent traffic, and classify prompt presence. Rebuilds dashboards into lightweight instrument panels with new donut/radar charts, model‑usage share, multi‑language GEO prompts, and faster cached queries.
New Features
@notra/toolsto read social overview, top posts, engagement timeseries, posting performance, GEO overview, and create/read A/B tests.@notra/beaconedge middleware with app ingest endpoint and proxy wiring; dashboard card with purpose donut plus request log.@notra/ui, and instrument grid/module/reveal for the new panel layout.@notra/analyticsto speed charts.Bug Fixes
Written for commit 50839b4. Summary will update on new commits.
Summary by Comp AI
No blocking issues found.
Written for commit
50839b4. New commits will trigger a re-review. Generated by Comp AI.