feat: declare readOnlyHint and idempotentHint on all five tools (fixes #1) - #5
feat: declare readOnlyHint and idempotentHint on all five tools (fixes #1)#5Mayur021 wants to merge 1 commit into
Conversation
…GenAI-Security-Project#1) Signed-off-by: Mayur Agnihotri <mayur.agnihotri0021@gmail.com> Signed-off-by: Mayur Agnihotri <mayur.agnihotri0021@gmail.com>
|
Checked the spec on the one thing that differs between this and #4, and #4 is right. openWorldHint defaults to true (schema 2025-06-18 and 2026-07-28, same wording in both). So leaving it unset is not the neutral deferral I described in the issue, it asserts an open world, which is the opposite of what this server is. #4 setting it to false is correct and this PR is wrong on that point. While confirming that, one thing neither of us got right. The spec annotates idempotentHint with "(This property is meaningful only when readOnlyHint == false)". Both PRs set readOnlyHint: true and idempotentHint: true together, so the second is inert by definition. It is harmless, but it is also a claim the spec says carries no meaning in this position. That makes the correct block: annotations: {
readOnlyHint: true,
openWorldHint: false,
},Happy to close this in favour of #4 with idempotentHint dropped, or to push that change here, whichever the maintainers prefer. #4 is the closer starting point. |
|
Agreed on both points — thanks for pulling the exact schema wording.
Applied to #4 in bdece5a: all five tools now carry exactly annotations: {
readOnlyHint: true,
openWorldHint: false,
},typecheck clean, wrangler deploy --dry-run clean. Happy for #5 to close in favour of #4 — it's the closer starting point and now matches the correct block. |
Closes #1.
The server registers five tools and none of them pass annotations. A client reading tools/list sees title, description and inputSchema, with nothing describing behaviour, so it cannot distinguish a read-only server from one that
simply does not say. The read-only property is real and documented, but it lives in the README where a client cannot read it.
This adds annotations to all five: readOnlyHint and idempotentHint, both true.
Accurate on every path, since each one is a GET against raw.githubusercontent.com or the GitHub trees API and returns the same result for the same arguments within the cache window.
I have deliberately left openWorldHint unset. The tools do reach an external host, but the world they reach is a single pinned repository, and which way that reads is the project's call rather than mine. Happy to add it either way.
Worth being clear about what this is not. An annotation is a declaration, not a control, and the specification is right that clients must treat annotations as untrusted unless the server is. The clause that applies here is the second one:
a first-party server published by the project itself is the case that exception exists for. This does not make the server safer. It lets a well-behaved client tell that it is read-only, which today it cannot, so a client that fails closed
on undeclared behaviour has to treat this corpus reader like an unknown third-party server.
Additive only: five object literals, no type or signature changes. Authored through the web editor, so I have not run the typecheck locally.
Note on overlap with #3: that PR adds a sixth tool, get_corpus_revision. If it merges first I will add the same two annotations to it in a follow-up, or rebase this onto it, whichever the maintainers prefer.