googleapis / googleapis/mcp-toolbox

Oracle `readOnly: true` does not prevent writes: UPDATE/DELETE run and commit

Open
#3,987 1 comment 0 reactions 1 assignee Claimed by @anubhav756 View on GitHub
priority: p1 type: bug
Dominant language
Go
Stars
16.4k
Forks
1.7k
Avg merge
4d 9h
Merged PRs (30d)
85

Description

### Prerequisites

- [x] I've searched the current open issues
- [x] I've updated to the latest version of Toolbox

### Toolbox version

`toolbox version 1.10.0+dev.linux.amd64`

### Environment

1. OS type and version: `Linux 6.8.0-106-generic #106~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC x86_64` (Ubuntu 22.04.5 LTS)
2. How are you running Toolbox: Compiled from source — `go build -o toolbox .`
3. Database: Oracle Database Free 23 (`gvenzl/oracle-free:slim`), PDB `FREEPDB1`

### Client

1. Client: plain HTTP against the MCP endpoint (`curl`), so this does not depend on any SDK
2. Version: n/a
3. Configuration used:

```yaml
sources:
my-oracle:
type: oracle
connectionString: 127.0.0.1:1521/FREEPDB1
user: toolbox
password: ${PASSWORD}
tools:
readonly-update:
type: oracle-sql
source: my-oracle
readOnly: true
description: A tool declared read-only that performs an UPDATE.
statement: UPDATE repro SET "name" = 'HACKED' WHERE "id" = 1
```

### Expected Behavior

A tool configured with `readOnly: true` should not be able to modify data. A
write statement should be rejected, not executed.

### Current Behavior

`readOnly` only selects `QueryContext` over `ExecContext`. That is a choice of Go
function, not a constraint on the database: both send the same SQL over the same
connection, and Oracle executes whatever it is sent. Since both drivers run in
autocommit, the write is committed immediately.

The two tools return different response shapes, which confirms the field is
parsed and honored:

```
readOnly: false -> {"content":[{"text":"{\"rows_affected\":1,\"status\":\"success\"}"}]}
readOnly: true -> {"content":[]}
```

But both wrote. Read back from a separate sqlplus session after each call:

```
after control-update (readOnly: false) -> WRITTEN_BY_RO_FALSE
after readonly-update (readOnly: true) -> WRITTEN_BY_RO_TRUE
```

So the `readOnly: true` call reports a successful, empty result while silently
committing the write — nothing in the response indicates anything was modified.

The write is durable, not pending on a pooled connection: after killing Toolbox
entirely and reading from a fresh session, the change is still there, along with
a row inserted the same way by a second `readOnly: true` tool:

```
id name
1 WRITTEN_BY_RO_TRUE
99 INSERTED_BY_RO_TRUE
```

`UPDATE` was verified on **both** drivers — `useOCI: true` (godror) and
`useOCI: false` (go-ora, the default). `INSERT` and `DELETE` behave the same
(verified with `useOCI: true`).

Relatedly, `SELECT ... FOR UPDATE` also runs through a `readOnly: true` tool, and
on go-ora (the default) it leaves the resulting transaction and row locks open on
the pooled connection — `v$transaction` stays at 1, still held 30+ seconds later,
and other sessions get `ORA-00054` until Toolbox is restarted. Same root cause.

### Steps to reproduce?

1. Start Oracle: `docker run -d --name oracle-free -e ORACLE_PASSWORD= -p 1521:1521 gvenzl/oracle-free:slim`
2. Seed a row:
```sql
CREATE TABLE repro ("id" NUMBER PRIMARY KEY, "name" VARCHAR2(50));
INSERT INTO repro ("id","name") VALUES (1,'ORIGINAL');
COMMIT;
```
3. Save the config above as `tools.yaml`, then run `./toolbox --tools-file tools.yaml`.
4. Invoke the tool declared read-only:
```sh
curl -s -X POST http://localhost:5000/mcp -H 'Content-Type: application/json' \
-d '{"jsonrpc":"2.0","method":"tools/call","params":{"name":"readonly-update","arguments":{}},"id":1}'
```
It returns `{"content":[]}` — a successful, empty result.
5. From a separate session, observe the committed write:
```sql
SELECT "name" FROM repro WHERE "id" = 1; -- WRITTEN_BY_RO_TRUE
```
Stopping Toolbox first shows the change survives, so it is committed. Invoking
`control-update` instead shows the flag is read: it returns `rows_affected`
rather than `{"content":[]}`.

### Additional Details

The flag appears to have been introduced in #2323 to fix "incorrect array type"
errors when DML was forced through `QueryContext`, i.e. to choose the result
shape (rows vs. `rows_affected`). The name suggests a permission it never
enforced.

Closing this in Go by inspecting the statement text would not be reliable — any
client-side read-only guarantee based on pattern-matching SQL is a guess. Oracle
can enforce it directly with `SET TRANSACTION READ ONLY`, which rejects writes
and locking reads with `ORA-01456`.

A source-level option (the AlloyDB / Cloud SQL `readOnly` pattern, which pins a
session parameter at connect time) is not available here: Oracle has no
session-level read-only mode — `ALTER SESSION SET READ ONLY` fails with
`ORA-02248`. Transaction scope is the only mechanism Oracle offers.

I have a fix ready and will link a PR.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.