[astandrik]

What does this PR do?

The Graph UI already calls get_graph_schema while loading projects, but the loopback UI RPC allowlist rejected that call with HTTP 403.

This adds only get_graph_schema to the existing list_projects / get_code_snippet allowlist. The regression test now covers all three allowed tools and keeps search_graph, delete_project, manage_adr, ingest_traces, and index_repository blocked. The existing initialize and duplicate params.name checks remain unchanged.

Fixes #1663

Verification

• Reproduce-first HTTPD run: applying the test change alone fails the target allowlist case with HTTP 403.
• scripts/test.sh --suites httpd
• scripts/test.sh
• scripts/lint.sh --ci
• scripts/build.sh --with-ui BUILD_DIR=build/issue1663-ui CC=gcc CXX=g++
• SMOKE_REQUIRE_UI=1 scripts/smoke-local.sh build/issue1663-ui/codebase-memory-mcp ui
• make -j32 -f Makefile.cbm security BUILD_DIR=build/issue1663-security

The full Linux gate passed on the remote builder against the exact commit tree. Windows runtime behavior remains for CI to confirm.

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass (scripts/test.sh)
• Lint passes (scripts/lint.sh --ci)
• New behavior is covered by a reproduce-first regression test

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[Copilot]

🟢 Approval recommended

The change is narrowly scoped to an explicit allowlist addition with corresponding regression coverage for both allowed and blocked RPC methods.

Pull request overview

This PR fixes the Graph UI’s failure to fetch per-project graph schema over the loopback JSON-RPC endpoint by expanding the UI RPC allowlist to include get_graph_schema, and updates the HTTPD regression test to validate the expanded allowlist while keeping mutation/search tools blocked.

Changes:

• Allow get_graph_schema through the UI /rpc allowlist alongside list_projects and get_code_snippet.
• Expand the existing HTTPD regression test to assert all allowed tools return HTTP 200 and selected blocked tools still return HTTP 403.
• Add search_graph to the blocked-tool coverage to ensure graph query capabilities remain disallowed via the UI RPC route.

File summaries

File	Description
tests/test_httpd.c	Expands the UI RPC allowlist/denylist regression test to cover get_graph_schema and additional blocked tools.
src/ui/http_server.c	Adds get_graph_schema to the UI RPC allowlist check in rpc_is_allowed_for_ui.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[DeusData]

Thank you for isolating this to the UI RPC allowlist and for covering both the allowed and blocked methods. Adding get_graph_schema changes the loopback UI RPC security boundary, so we need more time to review the route guarantees and the complete allowlist before making a merge decision. We will come back with a decision; no additional work is requested from you at this stage.

[DeusData]

Approved on merit. All 24 red checks are one environmental cause and none of them are yours — details below, because 24 failures on a two-line change is alarming until you know why.

The change

This touches a security allowlist, so I looked at it as one. http_server.c:1757-1759 currently gates the loopback UI RPC on method == "tools/call" and name ∈ {list_projects, get_code_snippet}.

The argument that settles it: get_code_snippet is already allowed, and it returns actual source code. get_graph_schema returns node and edge type names. Adding it is strictly less exposure than what the allowlist already permits, over the same loopback-only channel, for a call the UI already makes and currently gets a 403 for.

Your test keeps search_graph, delete_project, manage_adr, ingest_traces and index_repository blocked, which is the half that matters — the mutating tools stay out, and the negative cases are what stop this allowlist growing by accident later.

Doing it reproduce-first, so the test change alone fails with the 403 before the source change, is exactly right for an allowlist addition: it proves the guard was actually the thing rejecting the call, rather than something else upstream.

Why 24 checks are red

Every failing leg dies at the same preflight, before any test runs:

ERROR: repository build/ ancestor must be absent or a real directory

That is scripts/path-safety.sh:30, which refuses to proceed when the repository's build/ is a symlink or exists as a non-directory. It is a guard against rm-ing outside the repo, and it fires on the state of the runner's working tree — not on anything in a diff. A two-line change to src/ui/http_server.c cannot produce it.

That single cause explains the whole board: every test-unix shard, every test-tsan, the sanitizer legs, pr-smoke on all three platforms, shard-completeness (its dependencies died), and ci-ok (it aggregates). One environmental fault, twenty-four red checks.

I have recorded the signature so this is recognised immediately next time rather than re-diagnosed.

Before merge

You are only 8 commits behind main, which is the freshest of anything in my queue today — thank you for keeping it current.

I would normally rebase to trigger a clean re-run, but our Actions pool is in a bad state: there is exactly one job executing repo-wide against 24 queued runs, so every new run I start makes the wait worse for everyone. I am deliberately holding off and will refresh this once the pool recovers.

Your local verification list is unusually thorough — the UI build, SMOKE_REQUIRE_UI=1 smoke-local.sh, and the security target — and saying plainly that "Windows runtime behavior remains for CI to confirm" is the right way to state a limit rather than imply coverage you do not have.

[DeusData]

An update that you are owed an honest explanation for, because this PR was approved three days ago. A direction decision has landed since: the graph UI is being reimplemented from the ground up, and the current app together with its loopback RPC layer — the allowlist this change edits — will be replaced wholesale rather than maintained. With that settled, merging a change into the surface that is about to disappear gives it a very short life, and we would rather be straight with you than land it and delete it in the next release cycle. So we are closing this one, with #1663 closed alongside it as superseded.

What carries forward: the stats panel's schema request is on the requirements list for the new version, and so is your point that the allowlist should be tested from both sides — the allowed and the blocked method — which was the part of this PR that made it a good one. Thank you for isolating the bug precisely and for a two-line fix with more test than change; that is exactly the shape we like, and you are very welcome back.