Skip to content

Fix server crashing when tracing certain schemas with references#811

Merged
jviotti merged 2 commits intomainfrom
trace-crash
Apr 3, 2026
Merged

Fix server crashing when tracing certain schemas with references#811
jviotti merged 2 commits intomainfrom
trace-crash

Conversation

@jviotti
Copy link
Copy Markdown
Member

@jviotti jviotti commented Apr 3, 2026

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark Index (community)

Details
Benchmark suite Current: 410e99c Previous: c20a47a Ratio
Add one schema (0 existing) 18 ms 18 ms 1
Add one schema (100 existing) 19 ms 23 ms 0.83
Add one schema (1000 existing) 66 ms 75 ms 0.88
Add one schema (10000 existing) 535 ms 631 ms 0.85
Update one schema (1 existing) 14 ms 17 ms 0.82
Update one schema (101 existing) 20 ms 24 ms 0.83
Update one schema (1001 existing) 62 ms 78 ms 0.79
Update one schema (10001 existing) 543 ms 645 ms 0.84
Cached rebuild (1 existing) 8 ms 9 ms 0.89
Cached rebuild (101 existing) 9 ms 11 ms 0.82
Cached rebuild (1001 existing) 21 ms 31 ms 0.68
Cached rebuild (10001 existing) 168 ms 248 ms 0.68
Index 100 schemas 87 ms 109 ms 0.80
Index 1000 schemas 707 ms 865 ms 0.82
Index 10000 schemas 11377 ms 27513 ms 0.41

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark Index (enterprise)

Details
Benchmark suite Current: 410e99c Previous: c20a47a Ratio
Add one schema (0 existing) 20 ms 23 ms 0.87
Add one schema (100 existing) 26 ms 24 ms 1.08
Add one schema (1000 existing) 73 ms 73 ms 1
Add one schema (10000 existing) 600 ms 688 ms 0.87
Update one schema (1 existing) 22 ms 18 ms 1.22
Update one schema (101 existing) 25 ms 24 ms 1.04
Update one schema (1001 existing) 74 ms 76 ms 0.97
Update one schema (10001 existing) 607 ms 625 ms 0.97
Cached rebuild (1 existing) 10 ms 10 ms 1
Cached rebuild (101 existing) 12 ms 11 ms 1.09
Cached rebuild (1001 existing) 32 ms 33 ms 0.97
Cached rebuild (10001 existing) 245 ms 297 ms 0.82
Index 100 schemas 107 ms 108 ms 0.99
Index 1000 schemas 1525 ms 998 ms 1.53
Index 10000 schemas 19156 ms 13831 ms 1.39

This comment was automatically generated by workflow using github-action-benchmark.

@jviotti jviotti force-pushed the trace-crash branch 2 times, most recently from 663a6a9 to d8559ed Compare April 3, 2026 18:07
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@jviotti jviotti marked this pull request as ready for review April 3, 2026 18:43
@jviotti jviotti changed the title [WIP] Fix server crashing when tracing certain schemas with references Fix server crashing when tracing certain schemas with references Apr 3, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/actions/schema_directory.cc">

<violation number="1" location="src/actions/schema_directory.cc:30">
P1: Strip all leading slashes from `uri_path`; stripping only one can still leave an absolute path and bypass the intended `output/schemas` prefix.</violation>

<violation number="2" location="src/actions/schema_directory.cc:38">
P0: Reject `uri_path` values containing absolute or parent (`..`) path components before building the filesystem path to prevent directory traversal.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

return std::nullopt;
}

return output / "schemas" / uri_path / "%";
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Reject uri_path values containing absolute or parent (..) path components before building the filesystem path to prevent directory traversal.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/actions/schema_directory.cc, line 38:

<comment>Reject `uri_path` values containing absolute or parent (`..`) path components before building the filesystem path to prevent directory traversal.</comment>

<file context>
@@ -0,0 +1,41 @@
+    return std::nullopt;
+  }
+
+  return output / "schemas" / uri_path / "%";
+}
+
</file context>
Fix with Cubic

uri_path = uri_path.substr(base_path.size());
}

if (uri_path.starts_with('/')) {
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Strip all leading slashes from uri_path; stripping only one can still leave an absolute path and bypass the intended output/schemas prefix.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/actions/schema_directory.cc, line 30:

<comment>Strip all leading slashes from `uri_path`; stripping only one can still leave an absolute path and bypass the intended `output/schemas` prefix.</comment>

<file context>
@@ -0,0 +1,41 @@
+    uri_path = uri_path.substr(base_path.size());
+  }
+
+  if (uri_path.starts_with('/')) {
+    uri_path = uri_path.substr(1);
+  }
</file context>
Fix with Cubic

@jviotti jviotti merged commit 588a769 into main Apr 3, 2026
6 checks passed
@jviotti jviotti deleted the trace-crash branch April 3, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant