Skip to content

Commit 634fa94

Browse files
authored
fix & impr (core + hql + cli): gateway permissions checking, cli rendering issues, default values for edges and vecs, custom return structures (#692)
## Description <!-- Provide a brief description of the changes in this PR --> ## Related Issues <!-- Link to any related issues using #issue_number --> Closes # ## Checklist when merging to main <!-- Mark items with "x" when completed --> - [ ] No compiler warnings (if applicable) - [ ] Code is formatted with `rustfmt` - [ ] No useless or dead code (if applicable) - [ ] Code is easy to understand - [ ] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [ ] All tests pass - [ ] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes <!-- Add any additional information that would be helpful for reviewers --> <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR merges the `dev` branch into `main`, introducing API key verification, vector database enhancements, query generator improvements, and various bug fixes. ## Major Changes - **API Key Authentication**: Added optional API key verification using SHA-256 hashing and constant-time comparison (feature-gated with `api-key` flag) - **Vector Core Enhancements**: Implemented `get_all_vectors` method with level filtering and improved error messages - **Query Generator**: Enhanced object literal construction, nested struct handling, and closure variable resolution for complex query patterns - **Builtin Handlers**: Updated to use arena allocators for improved memory management - **Test Coverage**: Added comprehensive integration tests for API key verification ## Issues Found - **Critical**: `gateway.rs:178` uses `.unwrap()` on `api_key_hash` which could panic if the field is unexpectedly None (though this should be prevented by request validation when the feature is enabled) <details><summary><h3>Important Files Changed</h3></summary> File Analysis | Filename | Score | Overview | |----------|-------|----------| | helix-db/src/helix_gateway/key_verification.rs | 4/5 | New API key verification module with constant-time comparison using `subtle` crate, comprehensive test coverage | | helix-db/src/helix_gateway/gateway.rs | 3/5 | Added API key verification to `post_handler`, uses `.unwrap()` which could panic, follows OnceLock const block rule correctly | | helix-db/src/protocol/request.rs | 5/5 | Added `api_key_hash` field to Request struct, hashes incoming x-api-key header with SHA-256, comprehensive tests added | | helix-db/src/protocol/error.rs | 5/5 | Added `InvalidApiKey` error variant with 403 HTTP status code, proper error handling and tests | | helix-db/src/helix_engine/vector_core/vector_core.rs | 5/5 | Added `get_all_vectors` method with optional level filtering, improved error messages with uuid_str utility | | helix-db/src/helixc/generator/queries.rs | 4/5 | Enhanced query generator with object literal construction, nested struct handling, and improved closure variable resolution | </details> </details> <details><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant Client participant Gateway as Helix Gateway<br/>(post_handler) participant Request as Request Extractor<br/>(from_request) participant KeyVerify as key_verification<br/>(verify_key) participant WorkerPool as Worker Pool participant Response as HTTP Response Client->>Gateway: POST /{query_name}<br/>Header: x-api-key Gateway->>Request: Extract request alt api-key feature enabled Request->>Request: SHA-256 hash x-api-key header Request->>Request: Store hash in api_key_hash field else api-key feature disabled Request->>Request: Set api_key_hash = None end Request->>Gateway: Return Request object alt api-key feature enabled Gateway->>Gateway: unwrap() api_key_hash Gateway->>KeyVerify: verify_key(hash) KeyVerify->>KeyVerify: Constant-time comparison<br/>with stored hash alt Invalid key KeyVerify->>Gateway: Err(InvalidApiKey) Gateway->>Gateway: Log metrics event Gateway->>Response: 403 Forbidden Response->>Client: Error response else Valid key KeyVerify->>Gateway: Ok() end end Gateway->>WorkerPool: process(request) WorkerPool->>WorkerPool: Execute query WorkerPool->>Gateway: Result alt Success Gateway->>Gateway: Log success metrics Gateway->>Response: 200 OK with data else Error Gateway->>Gateway: Log error metrics Gateway->>Response: Error status code end Response->>Client: HTTP Response ``` </details> <!-- greptile_other_comments_section --> **Context used:** - Rule from `dashboard` - When using OnceLock::new() in Rust code, wrap it in a `const { ... }` block to satisfy clippy lints.... ([source](https://app.greptile.com/review/custom-context?memory=7f3c5d9b-5a5e-41cd-9e7e-1992245f637c)) <!-- /greptile_comment -->
2 parents 68b0c05 + 583e4cb commit 634fa94

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+3817
-474
lines changed

.github/workflows/db_tests.yml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ jobs:
1111
strategy:
1212
matrix:
1313
os: [ubuntu-latest, windows-latest, macos-latest]
14-
14+
env:
15+
HELIX_API_KEY: "12345678901234567890123456789012"
16+
1517
steps:
1618
- uses: actions/checkout@v4
1719

@@ -33,3 +35,13 @@ jobs:
3335
run: |
3436
cd helix-db
3537
cargo test --release --lib -- --skip concurrency_tests
38+
39+
- name: Run dev instance tests
40+
run: |
41+
cd helix-db
42+
cargo test --release --lib --features dev-instance -- --skip concurrency_tests
43+
44+
- name: Run production tests
45+
run: |
46+
cd helix-db
47+
cargo test --release --lib --features production -- --skip concurrency_tests

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ hack/
88
.DS_Store
99
/tmp
1010
/data
11-
claude.md
11+
claude.md
12+
.cargo/

Cargo.lock

Lines changed: 10 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

helix-cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "helix-cli"
3-
version = "2.1.1"
3+
version = "2.1.2"
44
edition = "2024"
55

66
[dependencies]

helix-cli/src/commands/build.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ use crate::docker::DockerManager;
33
use crate::metrics_sender::MetricsSender;
44
use crate::project::{ProjectContext, get_helix_repo_cache};
55
use crate::utils::{
6-
copy_dir_recursive_excluding, helixc_utils::collect_hx_files, print_status, print_success,
6+
copy_dir_recursive_excluding, diagnostic_source, helixc_utils::collect_hx_files, print_status,
7+
print_success,
78
};
89
use eyre::Result;
910
use std::time::Instant;
@@ -24,8 +25,7 @@ use helix_db::{
2425
},
2526
},
2627
};
27-
use std::fmt::Write;
28-
use std::fs;
28+
use std::{fmt::Write, fs};
2929

3030
// Development flag - set to true when working on V2 locally
3131
const DEV_MODE: bool = cfg!(debug_assertions);
@@ -329,7 +329,7 @@ fn compile_helix_files(
329329
};
330330

331331
// Run static analysis
332-
let mut analyzed_source = analyze_source(source)?;
332+
let mut analyzed_source = analyze_source(source, &content.files)?;
333333

334334
// Read and set the config from the instance workspace
335335
analyzed_source.config = read_config(instance_src_dir)?;
@@ -371,7 +371,7 @@ fn parse_content(content: &Content) -> Result<Source> {
371371

372372
/// Runs the static analyzer on the parsed source to catch errors and generate diagnostics if any.
373373
/// Otherwise returns the generated source object which is an IR used to transpile the queries to rust.
374-
fn analyze_source(source: Source) -> Result<GeneratedSource> {
374+
fn analyze_source(source: Source, files: &[HxFile]) -> Result<GeneratedSource> {
375375
if source.schema.is_empty() {
376376
let error = crate::errors::CliError::new("no schema definitions found in project")
377377
.with_hint("add at least one schema definition like 'N::User { name: String }' to your .hx files");
@@ -384,7 +384,8 @@ fn analyze_source(source: Source) -> Result<GeneratedSource> {
384384
let mut error_msg = String::new();
385385
for diag in diagnostics {
386386
let filepath = diag.filepath.clone().unwrap_or("queries.hx".to_string());
387-
error_msg.push_str(&diag.render(&source.source, &filepath));
387+
let snippet_src = diagnostic_source(&filepath, files, &source.source);
388+
error_msg.push_str(&diag.render(snippet_src.as_ref(), &filepath));
388389
error_msg.push('\n');
389390
}
390391
return Err(eyre::eyre!("Compilation failed:\n{error_msg}"));

helix-cli/src/commands/check.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ fn validate_project_syntax(project: &ProjectContext) -> Result<()> {
6868
}
6969

7070
// Run static analysis to catch validation errors
71-
analyze_source(source)?;
71+
analyze_source(source, &content.files)?;
7272

7373
print_success("All queries and schema are valid");
7474
Ok(())

helix-cli/src/commands/compile.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub async fn run(output_dir: Option<String>, path: Option<String>) -> Result<()>
4141
}
4242

4343
// Run static analysis to catch validation errors
44-
let generated_source = analyze_source(source)?;
44+
let generated_source = analyze_source(source, &content.files)?;
4545

4646
// Generate Rust code
4747
let output_dir = output_dir

helix-cli/src/utils.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,26 @@
11
use crate::errors::CliError;
22
use color_eyre::owo_colors::OwoColorize;
33
use eyre::{Result, eyre};
4-
use std::fs;
5-
use std::path::Path;
4+
use helix_db::helixc::parser::types::HxFile;
5+
use std::{borrow::Cow, fs, path::Path};
66

77
const IGNORES: [&str; 3] = ["target", ".git", ".helix"];
88

9+
/// Resolve the source text to use when rendering diagnostics.
10+
pub fn diagnostic_source<'a>(
11+
filepath: &str,
12+
files: &'a [HxFile],
13+
fallback: &'a str,
14+
) -> Cow<'a, str> {
15+
if let Some(src) = files.iter().find(|file| file.name == filepath) {
16+
Cow::Borrowed(src.content.as_str())
17+
} else if let Ok(contents) = fs::read_to_string(filepath) {
18+
Cow::Owned(contents)
19+
} else {
20+
Cow::Borrowed(fallback)
21+
}
22+
}
23+
924
/// Copy a directory recursively without any exclusions
1025
pub fn copy_dir_recursively(src: &Path, dst: &Path) -> Result<()> {
1126
if !src.is_dir() {
@@ -233,8 +248,7 @@ pub mod helixc_utils {
233248
types::{Content, HxFile, Source},
234249
},
235250
};
236-
use std::fs;
237-
use std::path::Path;
251+
use std::{fs, path::Path};
238252

239253
/// Collect all .hx files from queries directory and subdirectories
240254
pub fn collect_hx_files(root: &Path, queries_dir: &Path) -> Result<Vec<std::fs::DirEntry>> {
@@ -302,13 +316,14 @@ pub mod helixc_utils {
302316
}
303317

304318
/// Analyze source for validation (similar to build.rs)
305-
pub fn analyze_source(source: Source) -> Result<GeneratedSource> {
319+
pub fn analyze_source(source: Source, files: &[HxFile]) -> Result<GeneratedSource> {
306320
let (diagnostics, generated_source) =
307321
analyze(&source).map_err(|e| eyre::eyre!("Analysis error: {}", e))?;
308322

309323
if !diagnostics.is_empty() {
310324
// Format diagnostics properly using the helix-db pretty printer
311-
let formatted_diagnostics = format_diagnostics(&diagnostics, &generated_source.src);
325+
let formatted_diagnostics =
326+
format_diagnostics(&diagnostics, &generated_source.src, files);
312327
return Err(eyre::eyre!(
313328
"Compilation failed with {} error(s):\n\n{}",
314329
diagnostics.len(),
@@ -323,6 +338,7 @@ pub mod helixc_utils {
323338
fn format_diagnostics(
324339
diagnostics: &[helix_db::helixc::analyzer::diagnostic::Diagnostic],
325340
src: &str,
341+
files: &[HxFile],
326342
) -> String {
327343
let mut output = String::new();
328344
for diagnostic in diagnostics {
@@ -332,7 +348,8 @@ pub mod helixc_utils {
332348
.clone()
333349
.unwrap_or("queries.hx".to_string());
334350

335-
output.push_str(&diagnostic.render(src, &filepath));
351+
let snippet_src = super::diagnostic_source(&filepath, files, src);
352+
output.push_str(&diagnostic.render(snippet_src.as_ref(), &filepath));
336353
output.push('\n');
337354
}
338355
output

helix-container/src/queries.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
21
// DEFAULT CODE
32
use helix_db::helix_engine::traversal_core::config::Config;
43

54
pub fn config() -> Option<Config> {
65
None
7-
}
6+
}

helix-db/Cargo.toml

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "helix-db"
3-
version = "1.1.1"
3+
version = "1.1.2"
44
edition = "2024"
55
description = "HelixDB is a powerful, open-source, graph-vector database built in Rust for intelligent data storage for RAG and AI."
66
license = "AGPL-3.0"
@@ -57,6 +57,8 @@ polars = { version = "0.46.0", features = [
5757
"lazy",
5858
"json",
5959
], optional = true }
60+
subtle = "2.6.1"
61+
sha_256 = "=0.1.1"
6062

6163
[dev-dependencies]
6264
rand = "0.9.0"
@@ -73,7 +75,7 @@ compiler = ["pest", "pest_derive"]
7375

7476
# vector features
7577
cosine = []
76-
78+
api-key = []
7779
build = ["compiler"]
7880
vectors = ["cosine", "url"]
7981
server = ["build", "compiler", "vectors", "reqwest"]
@@ -82,13 +84,4 @@ bench = ["polars"]
8284
dev = ["debug-output", "server", "bench"]
8385
dev-instance = []
8486
default = ["server"]
85-
86-
# benches/tests
87-
[[test]]
88-
name = "bm25_benches"
89-
path = "benches/bm25_benches.rs"
90-
91-
[[test]]
92-
name = "hnsw_benches"
93-
path = "benches/hnsw_benches.rs"
94-
87+
production = ["api-key","server"]

0 commit comments

Comments
 (0)