From ffa5d84dc972a0e94cc8600b56b388def4e27867 Mon Sep 17 00:00:00 2001 From: srpatcha Date: Tue, 25 Aug 2026 00:16:06 -0700 Subject: [PATCH] chore(ci): remove Dependabot, add Claude code review Dependabot is disabled org-wide: config removed here, and alerts plus automated security fixes turned off via the API. 90 open Dependabot PRs were closed. Note this also stops CVE alerts for dependencies. Adds .github/workflows/claude-code-review.yml. It complements the automatic Copilot review now enforced on the default branch: Copilot does the first pass on every PR, Claude runs on demand via @claude or the deep-review label. Needs an ANTHROPIC_API_KEY secret. Also includes audit fixes: Integer underflow in the GGUF parser produced a ~1.8e19 malloc size on a truncated file. Fixed a leak in the adaptive-profile test. CI set -DBUILD_TESTS=ON but the option is EAI_BUILD_TESTS, so it ran zero tests and passed. Co-Authored-By: Claude Opus 5 --- .github/dependabot.yml | 75 ------------- .github/workflows/ci.yml | 6 +- .github/workflows/claude-code-review.yml | 132 +++++++++++++++++++++++ TASKS.md | 4 +- formats/src/gguf_loader.c | 50 ++++++--- tests/test_adaptive.c | 2 + 6 files changed, 178 insertions(+), 91 deletions(-) delete mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/claude-code-review.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml deleted file mode 100644 index ce6d7dd..0000000 --- a/.github/dependabot.yml +++ /dev/null @@ -1,75 +0,0 @@ -# Standard, org-uniform Dependabot configuration template. -# -# Each repo should drop this file at .github/dependabot.yml and uncomment the -# ecosystem entries that apply. Keep weekly cadence so review backlogs stay -# manageable. Auto-assigning all PRs to a single triage owner (@srpatcha) -# prevents the "ten people CC'd, no one acts" failure mode. -# -# Reference template lives at embeddedos-org/.github/.github/dependabot-template.yml. - -version: 2 -updates: - # GitHub Actions — every repo should keep this enabled. - - package-ecosystem: github-actions - directory: / - schedule: - interval: weekly - day: monday - open-pull-requests-limit: 5 - assignees: - - srpatcha - labels: - - dependencies - - github-actions - - # Python — uncomment in repos with pyproject.toml or requirements.txt. - # - package-ecosystem: pip - # directory: / - # schedule: - # interval: weekly - # day: monday - # open-pull-requests-limit: 5 - # assignees: - # - srpatcha - # labels: - # - dependencies - # - python - - # Node.js — uncomment in repos with package.json. - # - package-ecosystem: npm - # directory: / - # schedule: - # interval: weekly - # day: monday - # open-pull-requests-limit: 5 - # assignees: - # - srpatcha - # labels: - # - dependencies - # - npm - - # Go — uncomment in repos with go.mod. - # - package-ecosystem: gomod - # directory: / - # schedule: - # interval: weekly - # day: monday - # open-pull-requests-limit: 5 - # assignees: - # - srpatcha - # labels: - # - dependencies - # - go - - # Docker — uncomment in repos with Dockerfile. - - package-ecosystem: docker - directory: / - schedule: - interval: weekly - day: monday - open-pull-requests-limit: 3 - assignees: - - srpatcha - labels: - - dependencies - - docker diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21d8b0a..8949033 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,9 +26,11 @@ jobs: cmake -B build -G Ninja \ -DCMAKE_C_COMPILER=gcc-12 \ -DCMAKE_CXX_COMPILER=g++-12 \ - -DBUILD_TESTS=ON + -DEAI_BUILD_TESTS=ON cmake --build build --parallel $(nproc) - cd build && ctest --output-on-failure || true + # No `|| true`: a failing test must fail the job. --no-tests=error + # catches an empty test set, which ctest otherwise reports as success. + cd build && ctest --output-on-failure --no-tests=error # ── Python Tests ────────────────────────────────────────────────────────── test-python: diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml new file mode 100644 index 0000000..aa02b2b --- /dev/null +++ b/.github/workflows/claude-code-review.yml @@ -0,0 +1,132 @@ +# Claude code review. +# +# This complements — it does not replace — the automatic Copilot review that +# runs on every PR via the "Code Quality Copilot review for default branch" +# ruleset. Copilot does the fast first pass; Claude does the deeper one, and +# only when asked, so routine PRs cost nothing. +# +# Two ways in: +# 1. Mention @claude in an issue, a PR comment, or a review comment. +# 2. Add the `deep-review` label to a PR for a full review pass. +# +# Requires an ANTHROPIC_API_KEY secret (org-level is easiest — one secret +# covers every repo). Without it both jobs fail fast with a clear message +# rather than reviewing silently with no credentials. +name: Claude Code Review + +on: + issue_comment: + types: [created] + pull_request_review_comment: + types: [created] + pull_request_review: + types: [submitted] + issues: + types: [opened, assigned, labeled] + pull_request: + types: [labeled] + +# Never run two reviews on the same PR at once; a new trigger supersedes the +# one in flight. +concurrency: + group: claude-review-${{ github.event.pull_request.number || github.event.issue.number || github.ref }} + cancel-in-progress: true + +jobs: + # ── 1. On-demand: someone wrote @claude ────────────────────────────────── + mention: + if: | + github.event_name != 'pull_request' && ( + (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || + (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || + (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || + (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) + ) + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: read + pull-requests: write + issues: write + id-token: write + steps: + - name: Check credentials + env: + KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + if [ -z "$KEY" ]; then + echo "::error::ANTHROPIC_API_KEY is not set. Add it as an organisation or repository secret." + exit 1 + fi + + - uses: actions/checkout@v6 + with: + fetch-depth: 1 + + - uses: anthropics/claude-code-action@v1 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + use_sticky_comment: true + + # ── 2. Full pass: PR labelled `deep-review` ────────────────────────────── + deep-review: + if: github.event_name == 'pull_request' && github.event.label.name == 'deep-review' + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + pull-requests: write + id-token: write + steps: + - name: Check credentials + env: + KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + if [ -z "$KEY" ]; then + echo "::error::ANTHROPIC_API_KEY is not set. Add it as an organisation or repository secret." + exit 1 + fi + + - uses: actions/checkout@v6 + with: + fetch-depth: 1 + + - uses: anthropics/claude-code-action@v1 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + use_sticky_comment: true + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + Review this pull request. The PR branch is already checked out in + the working directory. + + This repository follows the standards in its own CLAUDE.md, + QUALITY.md, TESTING.md, SECURITY.md and VERIFY.md — read the ones + that apply to the changed files and review against them, not + against generic style preferences. + + Prioritise, in this order: + 1. Correctness — logic errors, unhandled failure paths, integer + overflow/underflow, memory safety, race conditions. + 2. Security — unvalidated input crossing a boundary, injection, + authorization that checks only authentication, secrets in + source or logs. + 3. Tests — does a new behaviour have a test that would actually + fail without the fix? Flag assertions that cannot fail, and + tests that only check the negative case. + 4. Maintainability — only where it genuinely impedes a reader. + + Report what you verified and what you could not. Say plainly when + a concern is unverified rather than implying you checked it. If + nothing needs changing, say so in one line — do not manufacture + findings. + + Use `gh pr comment` for top-level feedback. + Use `mcp__github_inline_comment__create_inline_comment` (with + `confirmed: true`) for specific lines. + Only post GitHub comments — do not return review text as a message. + + claude_args: | + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr checks:*)" diff --git a/TASKS.md b/TASKS.md index b1e75f0..b9cbee1 100644 --- a/TASKS.md +++ b/TASKS.md @@ -17,7 +17,9 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`. | ID | Task | Owner | Verified by | Evidence | |----|------|-------|-------------|----------| -| — | None yet. | — | — | — | +| T-001 | CI ran zero tests and reported success | testing | reviewer | `ci.yml` configured with `-DBUILD_TESTS=ON`, but this project's option is `EAI_BUILD_TESTS`, so no test binary was built; `ctest` exits 0 on an empty test set (verified: exit code 0). The step also ended in `|| true`, which would have discarded a real failure as well. Fixed the flag, removed `|| true`, added `--no-tests=error`. CI now builds and runs 24 tests, all passing. | +| T-002 | Fix an integer underflow in the GGUF model-file parser | security | reviewer | `formats/src/gguf_loader.c` rounded the data-section offset up to a 32-byte boundary, which on a truncated file lands past EOF. `file_end - data_start` was then negative and cast to `size_t`, so `ctx->data_size` became 0xffffffffffffffe4 (~1.8e19). That value reached `malloc()` — ASan: `allocation-size-too-big` — and, because the `if (ctx->data)` guard left the field set, stayed in `ctx->data_size` for every later consumer to read as a length. Model files are attacker-supplied input. Now bounds-checked against EOF, with `data_size` set from the actual `fread` return so a short read cannot report more bytes than were read. `eai_format_tests` and `eai_format_ext_tests` pass under ASan/UBSan. | +| T-003 | Fix a memory leak in the adaptive-profile test | testing | reviewer | `tests/test_adaptive.c:305` loaded the `adaptive-edge` profile, which `strdup()`s seven tool names plus a provider string, and never called `eai_config_free()`. LeakSanitizer reported the direct leaks from `config.c:153` and `:158`. 24/24 eAI tests now pass under `-fsanitize=address,undefined`. | --- diff --git a/formats/src/gguf_loader.c b/formats/src/gguf_loader.c index 6480ff2..d033df4 100644 --- a/formats/src/gguf_loader.c +++ b/formats/src/gguf_loader.c @@ -6,6 +6,7 @@ #include #include #include +#include #define LOG_MOD "gguf" @@ -134,25 +135,48 @@ eai_status_t eai_gguf_load(const char *path, gguf_context_t *ctx) } } - /* Data section starts after alignment to 32 bytes */ - long pos = ftell(fp); - long aligned_pos = (pos + 31) & ~31L; - fseek(fp, aligned_pos, SEEK_SET); + /* Position reached after the header/KV/tensor-info sections. */ + long ftell_after_metadata = ftell(fp); - /* Read tensor data */ - long data_start = ftell(fp); - fseek(fp, 0, SEEK_END); + /* Establish the true end of file first, so the data section can be + * bounds-checked against it. */ + if (fseek(fp, 0, SEEK_END) != 0) { fclose(fp); eai_gguf_free(ctx); return EAI_ERR_IO; } long file_end = ftell(fp); + if (file_end < 0) { fclose(fp); eai_gguf_free(ctx); return EAI_ERR_IO; } + + /* Data section starts after alignment to 32 bytes */ + long pos = ftell_after_metadata; + if (pos < 0) { fclose(fp); eai_gguf_free(ctx); return EAI_ERR_IO; } + long aligned_pos = (pos > LONG_MAX - 31) ? file_end : ((pos + 31) & ~31L); + + /* Rounding up to the alignment boundary can land past the end of a + * truncated file. Computing file_end - data_start unguarded then yielded a + * negative value that became ~1.8e19 when cast to size_t, and that value + * reached malloc() and stayed in ctx->data_size for every later consumer to + * read as a length. Treat "starts at or past EOF" as an empty data section. */ + if (aligned_pos >= file_end) { + ctx->data = NULL; + ctx->data_size = 0; + fclose(fp); + return EAI_OK; + } + + long data_start = aligned_pos; ctx->data_size = (size_t)(file_end - data_start); - if (ctx->data_size > 0) { - ctx->data = (uint8_t *)malloc(ctx->data_size); - if (ctx->data) { - fseek(fp, data_start, SEEK_SET); - fread(ctx->data, 1, ctx->data_size, fp); - } + ctx->data = (uint8_t *)malloc(ctx->data_size); + if (!ctx->data) { ctx->data_size = 0; fclose(fp); eai_gguf_free(ctx); return EAI_ERR_NOMEM; } + + if (fseek(fp, data_start, SEEK_SET) != 0) { + free(ctx->data); ctx->data = NULL; ctx->data_size = 0; + fclose(fp); eai_gguf_free(ctx); return EAI_ERR_IO; } + /* Keep data_size consistent with what was actually read, so a short read + * cannot leave a length longer than the buffer's valid contents. */ + size_t got = fread(ctx->data, 1, ctx->data_size, fp); + ctx->data_size = got; + fclose(fp); return EAI_OK; } diff --git a/tests/test_adaptive.c b/tests/test_adaptive.c index 914bd1f..dae5a0e 100644 --- a/tests/test_adaptive.c +++ b/tests/test_adaptive.c @@ -307,6 +307,8 @@ TEST(test_adaptive_profile_loading) assert(cfg.adaptive.lora_rank == 8); assert(cfg.adaptive.max_training_memory_mb == 512); assert(cfg.tool_count == 7); + /* The profile strdup()s each tool name; without this the test leaks them. */ + eai_config_free(&cfg); return 0; }