The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 29, 2026 5:46am

Greptile Summary

This PR prevents GitHub tool parameters from redirecting authenticated requests through URL path traversal and adds regression coverage for path-building tools.

• Introduces distinct guards for opaque segments, slash-delimited paths, and encoded single-segment values.
• Applies the guards across GitHub request builders and safely encodes affected query parameters.
• Preserves whitespace in repository paths, accounts for discovery skips, and removes the prior unsafe any usage from the test harness.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported whitespace-path mutation, silent discovery skips, and unsafe test-harness typing have been addressed in the current code.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[User or LLM parameter] --> B{Parameter shape}
  B -->|Opaque identifier| C[safeUrlPathSegment]
  B -->|Slash-delimited path| D[safeUrlPath]
  B -->|Single value allowing slash| E[safeEncodedUrlPathSegment]
  C --> F[GitHub request URL]
  D --> F
  E --> F
  F --> G[Authenticated GitHub API request]

Reviews (3): Last reviewed commit: "fix(github): permit a whitespace-only pa..." | Re-trigger Greptile

@greptile All six threads are addressed, replied to individually, and resolved — pushed in 7d4d2e1. Re-review please.

Summary of what changed:

• P1 url-path.ts:250 (segment trimming) — real, and the most important finding here. Reproduced it first: git tracks d/my file .txt and d/ lead.txt verbatim, so trimming made update_file/delete_file act on a different existing file. Rather than delete trimming outright, the behaviour is now split by purpose: safeUrlPathSegment keeps trimming (opaque copy-pasted ids; ~690 call sites depend on it), safeUrlPath trims nowhere and preserves whitespace byte-for-byte. The dot-segment check is unweakened — the URL parser removes %2e%2e but leaves %20..%20 inert. Split explained in TSDoc on both helpers and pinned in both directions.
• P2 discovery skipping — enabling the assertion found a real hole: github_job_logs had silently fallen out of the suite entirely, because a string filler in the sibling job_id parameter aborted the build before owner/repo could be probed. Fixed, and skips are now an explicit asserted ledger.
• P2 ToolConfig<any, any> — replaced with a structural interface plus an unknown type guard. One documented cast remains at the single narrowing point, because the harness intentionally feeds values the tools' own param types forbid, which is the test's whole point.
• cubic P2 operations.ts 500→400 — valid; path-guard failures now raise GitHubOperationError(400). Applied to pullRequestUrl as well, which had the same problem and was not flagged.

Every new pin was verified to fail against the pre-fix code before being committed. Gates: lint, check:audits 39/39, check-block-registry, type-check, and the full GitHub suite (12080 passing) are green.

Carrying over a review finding from #7269, which is rebased onto this branch. cubic raised it against url-path.ts there, but it is this PR's code, so it belongs here rather than riding in on the rebase.

P2: safeEncodedUrlPathSegment accepts backslashes that the path guards otherwise reject, allowing an encoded Windows-shaped traversal value to reach downstream URL handling. Reject backslashes before encoding, as safeUrlPath does.

My assessment: not a security issue, but a reasonable consistency question.

The backslash is inert here. I checked rather than assuming:

safeEncodedUrlPathSegment(...., name)  ->  %5C..%5C..
new URL(https://api.github.com/repos/o/r/issues/1/labels/ + that).pathname
  ->  /repos/o/r/issues/1/labels/%5C..%5C..

The value survives as one segment. The WHATWG parser removes a dot segment only when the segment is exactly . or .. (or their %2e spellings), and it does not decode %5C first, so %5C..%5C.. never becomes a path boundary. Same reasoning by which ..%2f..%2f is inert: encoding a separator is sufficient, and only the bare dot segment resists encoding — which this helper already rejects.

The asymmetry with safeUrlPath also looks deliberate to me. There, / is structure, so a backslash means a pasted Windows path that would silently address the wrong resource. safeEncodedUrlPathSegment encodes the whole value as one opaque segment, separators included, so there is no structural ambiguity — and a GitHub label literally named a\b is legal, so rejecting would break a valid value to prevent something that cannot happen.

Where cubic has a fair point: the two helpers read inconsistently side by side, and "rejects backslashes" is easier to reason about than "encodes them, and here is why that is safe." If you prefer uniform rejection for reviewability, the only cost is the a\b label case. Your call — flagging it so the decision is made here rather than lost.

Two more review findings from cubic against #7269 (rebased onto this branch), both in this PR's files. Routing them here rather than fixing them on the downstream PR.

1. safeEncodedUrlPathSegment backslash — cubic escalated this to P1

I raised this above as P2 and argued it was inert. cubic re-raised it at P1 with a different argument than the one I answered, and I now think it should just be fixed.

My original analysis still stands for the URL parser: safeEncodedUrlPathSegment('\..\..') yields %5C..%5C.., and the WHATWG parser does not decode %5C before dot-segment removal, so it never becomes a boundary. What I did not address is cubic's actual claim — "downstream path normalizers". Intermediaries are the gap: IIS-style normalization folds \ to /, and some proxies and WAFs do the same before the origin sees the path. %5C..%5C.. decoded and folded becomes /../... That is not the URL parser's behaviour, so my earlier check did not test for it and could not have.

The cost of rejecting is one exotic case — a GitHub label literally containing a backslash — against a defense-in-depth hole in a guard whose entire purpose is to be the last word on traversal. I'd take the rejection, matching safeUrlPath:

if (trimmed.includes('\\')) {
  throw new Error(`${paramName} cannot contain a backslash`)
}

That also removes the reviewability wart of two guards in one module disagreeing about backslashes, which is what drew the finding twice.

2. remove_label.ts — response is built from raw params while the request is guarded

Confirmed by reading it; this one is straightforwardly right:

// request path: guarded/trimmed
// transformResponse:
content:  `Label "${params.name}" removed from issue #${params.issue_number}`
html_url: `https://github.com/${params.owner}/${params.repo}/issues/${params.issue_number}`

With owner: ' octocat ' the DELETE correctly hits /repos/octocat/..., but the returned html_url is https://github.com/ octocat /repo/issues/1 — a broken link, and content echoes an untrimmed label name. The operation succeeds and reports itself with values that do not match what it did.

Worth a sweep rather than a one-line fix: any transformResponse in this PR that rebuilds a URL or confirmation string from raw params has the same split. Deriving the response from the same canonical values as the request is the fix.

For reference, #7269 hit the same class of bug and the shape of the fix there may be useful: safeUrlPathSegment accepts a numeric id (an LLM can send one as a JSON number), so a .trim() elsewhere in the tool threw a raw TypeError after the guard had passed. Rather than restate the normalization I derived the second use from the guard itself — decodeURIComponent(safeUrlPathSegment(value, name)) — so there is one rule instead of two that can drift.

Third finding from cubic against #7269, again in this PR's code. This one I think is a genuine bug that defeats a guard this PR just added, so worth prioritising.

executeGitHubTool does not await the comment operations, so buildGuardedUrl's 400 never fires

apps/sim/lib/internal/github/execute-tool.ts:

switch (request.toolId) {
  case 'github_comment':
    return executeToolOperationImplementation(executeGitHubCommentOperation, request)   // not awaited
  case 'github_comment_v2':
    return executeToolOperationImplementation(executeGitHubCommentV2Operation, request) // not awaited
  case 'github_latest_commit': {
    ...
    return Response.json(await getGitHubLatestCommit(...))                              // awaited
  }
}

return promise inside try completes the try block before the promise settles, so the enclosing catch never observes a rejection — the classic missing return await. The github_latest_commit branch already awaits; the two comment branches do not.

That matters specifically because of the mapping this PR introduces:

function buildGuardedUrl(build: () => string): string {
  try { return build() }
  catch (error) { throw new GitHubOperationError(getErrorMessage(error, 'Invalid GitHub request path'), 400) }
}

buildGuardedUrl converts a guard rejection into GitHubOperationError(…, 400) precisely so executeGitHubTool's catch can report it as a client error with the named "<param> cannot be …" message. For github_comment and github_comment_v2 that catch is unreachable, so an owner of .. does not produce the documented 400 — it escapes as an unhandled rejection or surfaces as a generic 500, which is the exact misattribution the TSDoc on buildGuardedUrl says it exists to prevent.

Fix is one keyword on each of the two branches:

return await executeToolOperationImplementation(executeGitHubCommentOperation, request)

Worth a test that a rejected guard value returns 400 rather than 500 through executeGitHubTool, since the current operations.test.ts exercises the guards directly and would not have caught this.

That is all three of cubic's findings against this PR's files now routed here: the safeEncodedUrlPathSegment backslash (escalated to P1, and I reversed my earlier position in favour of adding the rejection), remove_label.ts building its response from raw params, and this one. #7269 has none of these files in its diff; all its own findings are fixed and it is green.

Correction pushed in 515b951, from a finding cubic raised on #7269.

safeUrlPath no longer rejects a whitespace-only path component. I had it reject any segment that was empty after trimming. That check was wrong on both counts, and I verified both rather than taking it on faith.

Git permits it — a file and a directory whose entire name is spaces:

$ git ls-files | sed -n 'l'
d/   $
e/   /f.txt$

And rejecting it bought nothing. A whitespace-only segment is not a dot segment, and the parser never removes it:

new URL('https://x/a/%20%20%20/b').pathname  ->  /a/%20%20%20/b   (kept)
new URL('https://x/a/../b').pathname         ->  /b               (removed)

So the guard had no security value and a real cost: update_file threw before the PUT and a legitimate file became unreachable.

Now only a truly empty component is rejected — a //, where the caller wrote no name at all. ./.., backslashes, and every other guard are unchanged.

safeUrlPathSegment is untouched and still rejects an all-whitespace value. That asymmetry is deliberate, not an oversight: it trims opaque ids first, so one made only of spaces really has named nothing. Both halves are pinned, and the TSDoc now records why the check is absent — citing the git paths and the parser behaviour — so it is not reinstated later on aesthetic grounds.

This is the same error as the trimming bug one level down: reasoning about what a path segment ought to look like instead of what the filesystem and the URL parser actually do. Worth stating plainly since this helper is now shared.

New pins: docs/ /file.txt, a bare , e/ /f.txt, and d/ round-trip byte-for-byte, plus docs//file.txt still throws. I confirmed 29 tests go red if I restore the old check.

Dependency note for reviewers: #7269 branches off this PR and will need a rebase once this lands — it carries the older safeUrlPath and its own copy of this fix will conflict.

Gates re-run green: lint, check:audits 39/39, check-block-registry, type-check, and the full GitHub suite (12110 passing).

@greptile review

@cubic review

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

Fourth finding from cubic against #7269, again in this PR's code — and this one is a consequence of the whitespace fix itself, so worth deciding deliberately.

safeUrlPath rejects a whitespace-only path component

if (!segment.trim()) {
  throw new Error(`${paramName} cannot contain an empty or whitespace-only path segment`)
}

cubic's case: apps/sim/tools/github/update_file.ts, a path whose component is made only of spaces — docs/ /notes.md — now throws, so a valid GitHub file cannot be updated.

I think this is right, and it is the one inconsistency left in an otherwise coherent rule. The whitespace fix on this PR deliberately made the helper treat whitespace as data: it encodes the raw segment so docs/ draft.md is not silently rewritten to docs/draft.md, on the grounds that a leading or trailing space is a legal filename character git stores verbatim. A component that is entirely spaces is the same kind of value by the same argument — legal, storable, and the caller's own — yet it is now the single shape the helper refuses.

Suggested change, keeping the two rules distinct:

if (segment === '') {
  throw new Error(`${paramName} cannot contain an empty path segment`)
}

The important caveat: do not collapse this into "allow anything non-empty after trimming". The empty-segment rule exists for a different reason than the traversal rule — a genuinely empty component (a//b, a leading or trailing /) addresses a different object than the caller wrote, which is why #7269 adopted the rejection for Supabase storage keys. "a/ /b" and "a//b" must stay distinguishable; only the first should become legal.

I flagged this deviation when consolidating #7269 onto safeUrlPath and said then that if whitespace-only components should be preserved, the change belongs here rather than forked downstream. cubic has now produced the concrete GitHub case, so it is yours to take.

Running list of findings routed to this PR from #7269, all verified there:

1. safeEncodedUrlPathSegment accepts backslashes — escalated to P1; I reversed my earlier "inert" position, since the risk is downstream normalizers folding \ to /, not the WHATWG parser.
2. remove_label.ts builds html_url and content from raw params while the request path is guarded — a successful removal returns a dead link. Worth a sweep of every transformResponse that rebuilds a URL from raw params.
3. executeGitHubTool does not await the comment operations, so this PR's own buildGuardedUrl 400-mapping is unreachable for github_comment — a rejected owner surfaces as 500 rather than the documented 400.
4. This one.

For what it is worth, #7269 hit the same class as (3) on its own side — encodeStoragePath throwing into a generic 500 handler — and I fixed it there by mapping guard rejections to 400 at the call sites, with tests asserting no provider request is made.

Fifth finding routed from #7269 — and this one is the data-loss class that #7269 just had to fix twice, so flagging it with the pattern rather than just the instance.

safeEncodedUrlPathSegment trims, so a padded label name now removes a different label

safeEncodedUrlPathSegment does toGuardedString(value, paramName).trim(). GitHub label names were previously interpolated without a trim, so:

before: DELETE /repos/o/r/issues/1/labels/%20%20bug%20%20   -> 404, no-op
after:  DELETE /repos/o/r/issues/1/labels/bug               -> removes the real "bug" label

Two independent reasons this is wrong here:

1. A label name can legitimately contain surrounding whitespace. Unlike a UUID or a GCP project id, " bug " is a nameable GitHub label, so trimming addresses a different one. cubic's framing — preserve the untrimmed name, reject only the dot-segment spellings, percent-encode the rest — is right on plain correctness grounds.
2. It converts a failing request into a succeeding destructive one. Pre-PR the padded name matched nothing and the DELETE was a no-op.

The pattern, because it generalises past this one call site

#7269 hit reason (2) twice, and both were invisible until specifically looked for:

• BigQuery projectId on delete_dataset / delete_table: previously encodeURIComponent(params.projectId), never trimmed. " my-project " named no project and 404'd; trimming resolved it to the real project and irreversibly deleted a dataset or table. datasetId on the same tools was already trimmed, so projectId was the one identifier out of step — the guard hid its own hazard.
• Box Sign signRequestId on cancel_request: interpolated raw, so a padded id 404'd; trimming cancels a real signature request.

The rule that resolved both: guarding a path must not turn a failing request into a succeeding one. Operationally — for each parameter, ask whether this PR newly trims it, and whether it sits on an irreversible request. Refuse padding on the newly-trimmed ones; leave the already-trimmed ones alone, since refusing those breaks callers whose stored value works today.

Worth running that question across every parameter this PR guards, not just remove_label. Any identifier that was previously raw or bare-encodeURIComponent and now flows through safeUrlPathSegment or safeEncodedUrlPathSegment is in scope; the DELETE routes (delete_file, delete_release, delete_branch) are where it costs the most.

Label removal is reversible, so this is lower priority than the two above — but it is the same mechanism, and the sweep is the part I would not skip.

Three more from cubic against #7269, all in this PR's files — plus one that applies to your guards but was found on mine.

1. close_pr.ts — the data-loss class again, on owner/repo (P2)

When owner or repo has surrounding whitespace, this helper trims it and closes the unpadded repository's PR; before this change, GitHub received the padded path and returned 404.

This is the third instance of the pattern I described in my previous comment, and cubic is now explicitly recommending the fix #7269 landed. Pre-PR a padded owner produced %20%20octocat%20%20, matched no repository, and the PATCH was a no-op. safeUrlPathSegment trims, so it now closes a PR in the real repository.

strictUrlPathSegment in apps/sim/tools/strict-url-path.ts (on #7269, so it will exist once that merges) does exactly this — refuse padding on identifiers the change newly trims, leave already-trimmed ones alone.

2. get_branch_protection.ts — branch with / sent as extra path segments (P2)

safeUrlPath keeps / as structure, but branch protection takes the branch as one path parameter, so feature/foo becomes /branches/feature/foo/protection and 404s. This is what safeEncodedUrlPathSegment is for — worth auditing every branch/ref site for the same mix-up, since the two helpers are easy to swap by accident and the failure is a silent 404 rather than an error.

3. get_tree.ts — falsy 0 path skips the guard (P3)

params.path ? safeUrlPath(...) : treats the numeric 0 as absent, so a directory named 0 cannot be listed and malformed input silently becomes a root request. Needs an explicit "omitted or empty string" check rather than a truthiness test.

Also worth carrying over: guard errors echo the rejected value

cubic raised this on #7269 against my strictUrlPathSegment, and it applies to the guards here too — safeUrlPathSegment, safeUrlPath and safeEncodedUrlPathSegment all quote the offending value:

throw new Error(`${paramName} cannot be "${trimmed}" (path traversal is not allowed)`)

These parameters are visibility: 'user-or-llm' and buildGuardedUrl maps the throw into a tool result the model reads. So the message copies attacker-chosen text back into the prompt — from a value that arrived precisely because it was hostile enough to be rejected. U+2028/U+2029 is the concrete edge: they terminate a line for some parsers, so a rejected id can break out of whatever framing wraps the error.

I dropped the value from #7269's guard and kept only the parameter name, which is the actionable part since the caller already knows what it sent. Pinned with a payload shaped like the risk (' ignore previous instructions ') rather than a neutral string. Same change looks right here.

Running total routed from #7269: safeEncodedUrlPathSegment backslashes (P1), remove_label.ts raw-params response, executeGitHubTool missing await, safeUrlPath rejecting whitespace-only components, remove_label.ts trimming label names, and now these three plus the error-echo. Happy to take any of them as a follow-up PR against this branch if that is easier than folding them in.

Worked through all four. One I'm pushing back on, one I'm correcting the scope of, one is pre-existing, one isn't a defect. No code changes — this branch stays at 5/5.

Error-echo: does not apply to these guards

I verified this rather than taking it on faith, and I agree it does not apply here. The right question isn't "does the guard echo the value?" but "can the echoed value carry attacker-controlled text?" Every echoing site in url-path.ts is constrained to a value that cannot:

Every other throw names only paramName, which is a literal from the call site, never caller data.

Note :371 — that's a fourth echo site your list didn't include (safeEncodedUrlPathSegment). Same constant-only pattern, so the conclusion is unchanged, but the audit list should be complete or the next person will think one was missed.

I proved it rather than reasoning about it: a fuzz over 18 injection payloads (IGNORE PREVIOUS INSTRUCTIONS…, </result><system>…, {{prompt}}, a lone surrogate, U+2028, embedded newlines, a 200-char blob) plus 12 non-string/numeric shapes, across all three helpers — 90 cases. For every message produced, every quoted run is one of . or .., and no free-form payload appears anywhere in the text. U+2028 specifically cannot reach a message: a string containing it isn't a dot segment, so it exits via cannot contain a path separator or is encoded and returned.

The distinction matters in the other direction too: strictUrlPathSegment's echo is genuinely unsafe and your fix on #7269 is right, because it quotes free-form padded input. A guard that names a constant it just matched on is fine; one that quotes what the caller actually sent is not.

1. close_pr.ts — real, but it's 21 tools, not the third instance

The finding is correct: owner/repo were raw pre-PR, so I do newly introduce trimming, and a padded value that used to 404 now hits the real repository. That's the rule you stated, and it's met.

But scoping it to close_pr understates it. Every mutating GitHub tool had raw interpolation pre-PR, so all of them newly trim:

DELETE  delete_branch, delete_comment, delete_comment_reaction, delete_file,
        delete_issue_reaction, delete_milestone, delete_release, remove_label, unstar_repo
PATCH   close_issue, close_pr, update_comment, update_issue,
        update_milestone, update_pr, update_release
PUT     create_file, merge_pr, star_repo, update_branch_protection, update_file

21 tools. delete_gist is the one exception — it already had gist_id?.trim() pre-PR, so per your rule it isn't a change I'm making.

One nuance that shrinks the blast radius: the exposure is only via safeUrlPathSegment on owner/repo/ids. The path parameter on create_file/update_file/delete_file goes through safeUrlPath, which no longer trims at all — so the highest-stakes value in the destructive set is already immune as of 515b951.

I'm not fixing this here, deliberately. strictUrlPathSegment lands with #7269; hand-rolling a second copy on this branch would create two competing helpers and guarantee a conflict with the PR that introduces the real one — and it would fix 1 of 21. This wants one systematic pass over the whole table after #7269 merges. Filed as a follow-up.

2. get_branch_protection.ts — pre-existing, byte-identical, needs live-API verification

Not introduced by this PR. Pre-PR was raw ${params.branch}; safeUrlPath emits feature/foo unchanged — that's exactly why I chose it over safeEncodedUrlPathSegment for this parameter. Proven by the passing LEGITIMATE_PATHS assertion, which pins feature/my-branch through unchanged for every multi-segment param including this one. Same for get_branch, delete_branch, update_branch_protection, get_commit(ref), compare_commits(base/head).

So if /branches/feature/foo/protection 404s today, it 404'd identically before this PR.

Switching to %2F is a wire change on a currently-clean branch, and I'd want it verified against the live API first — GitHub's router is greedy for /branches/* on the plain endpoint, and whether that holds with a /protection suffix is the actual open question, not something to guess at. Worth noting it's also the mirror image of finding 1: there the objection is that a fix turns a failing request into a succeeding one. Follow-up, with verification.

3. get_tree.ts falsy 0 — not a defect

Pre-PR: params.path || ''. Post-PR: params.path ? safeUrlPath(...) : ''. Identical falsy set — 0 produced '' before and produces '' now. Nothing changed, so there's no regression to fix here.

Nor is the skipped guard a hole: every falsy value ('', 0, NaN, null, undefined, false) yields '', giving /contents/ — a repo-root listing. There is no value that skips the guard and reaches a path segment, so nothing traversable slips through. path is declared type: 'string' and optional here, which is why the truthiness test is the right shape; get_file_content guards unconditionally because its path is required. I'd rather not add a typeof/length dance to a branch at 5/5 for a case with no behavioural difference.

Happy to take the follow-ups — the 21-tool strictUrlPathSegment pass is the substantive one and should go in after #7269 so there's a single helper to call.

Tenth finding routed from #7269 — cubic, apps/sim/tools/github/get_tree.ts:

P2/P3: When ref contains a lone UTF-16 surrogate from a malformed JSON/LLM value, encodeURIComponent throws a bare URIError before the request can be prepared. Guard this encoding like encodeSegment so the tool returns a named ref input error instead of an opaque request failure.

Valid, and the fix already exists in this PR — it just is not used at that call site. encodeSegment in url-path.ts wraps exactly this:

function encodeSegment(segment: string, paramName: string): string {
  try {
    return encodeURIComponent(segment)
  } catch {
    throw new Error(`${paramName} contains an unpaired UTF-16 surrogate and cannot be encoded`)
  }
}

I verified the difference on the downstream branch with a lone high surrogate:

via encodeSegment (safeUrlPathSegment):  Error: projectId contains an unpaired UTF-16 surrogate and cannot be encoded
raw encodeURIComponent:                  URIError: URI malformed

So the guards that go through encodeSegment are already correct; the gap is anywhere this PR still calls encodeURIComponent directly. Worth grepping for that rather than patching get_tree.ts alone — the same shape will exist at any other direct call.

For symmetry with what I found downstream: #7269 has one such site, google_drive/get_content.ts interpolating encodeURIComponent(exportFormat) into a query parameter. I left it alone there because it is byte-identical in origin/staging, untouched by that diff, and a query param rather than a path segment — outside that PR's contract. Same judgement may or may not apply here depending on whether the get_tree.ts site is one this PR introduced.

Running list routed from #7269 (all verified there, none fixed downstream since they are this PR's files):

1. safeEncodedUrlPathSegment accepts backslashes — P1, I reversed my initial "inert" position; the risk is downstream normalizers folding \ to /.
2. remove_label.ts builds html_url/content from raw params — successful removal returns a dead link.
3. executeGitHubTool does not await its comment operations — makes this PR's own buildGuardedUrl 400-mapping unreachable.
4. safeUrlPath rejects a whitespace-only path component — over-rejects since the whitespace fix.
5. remove_label.ts trims label names — data-loss class; a label may legitimately carry whitespace.
6. close_pr.ts trims owner/repo — data-loss class; padded owner went from 404 no-op to closing a real PR.
7. get_branch_protection.ts splits a branch containing / — silent 404; wants safeEncodedUrlPathSegment.
8. get_tree.ts skips the guard for a falsy 0 path.
9. Guard errors echo the rejected value into a model-facing tool result — prompt-injection surface.
10. This one.

Offer stands: happy to take any subset as a follow-up PR against this branch if that is easier than folding them in.