Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/oci-compute-instance-agent-mcp-server/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixed

- Fixed pagination limits in list tools to ensure results respect the requested maximum across API pages.

### Security

- Updated `cryptography` to 50.0.1 to prevent PKCS#7 EnvelopedData decryption from exposing a Bleichenbacher oracle through distinguishable errors and timing (CVE-2026-69247).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,12 @@ def list_instance_agent_command_executions(
next_page: str = None

while has_next_page and (limit is None or len(commands) < limit):
remaining = None if limit is None else limit - len(commands)
kwargs = {
"compartment_id": compartment_id,
"instance_id": instance_id,
"page": next_page,
"limit": limit,
"limit": remaining,
Comment thread
dustin-sale marked this conversation as resolved.
}

response = client.list_instance_agent_command_executions(**kwargs)
Expand All @@ -202,8 +203,10 @@ def list_instance_agent_command_executions(

data: list[oci.compute_instance_agent.models.InstanceAgentCommandExecutionSummary] = response.data
for d in data:
commands.append(map_instance_agent_command_execution_summary(d))

if limit is None or len(commands) < limit:
commands.append(map_instance_agent_command_execution_summary(d))
else:
break
logger.info(f"Found {len(commands)} Instance Agent Commands")
return commands

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,20 +142,28 @@ async def test_list_instance_agent_commands(self, mock_get_client):
assert result[0]["instance_agent_command_id"] == mock_command_1.instance_agent_command_id

@pytest.mark.asyncio
@patch("oracle.oci_compute_instance_agent_mcp_server.server.get_compute_instance_agent_client")
async def test_list_instance_agent_commands_pagination_and_limit_and_output_types(self, mock_get_client):
# This test exercises:
# - pagination (has_next_page + next_page)
# - limit enforcement
# - mapping of different output content types (TEXT, OBJECT_STORAGE_URI, OBJECT_STORAGE_TUPLE)
@patch(
"oracle.oci_compute_instance_agent_mcp_server.server."
"get_compute_instance_agent_client"
)
async def test_list_instance_agent_commands_pagination_and_limit_and_output_types(
self, mock_get_client
):
# Exercises:
# - Pagination using has_next_page and next_page
# - Decreasing the requested limit across pages
# - Stopping when the overall result limit is reached,
# even when the last response advertises another page
# - Mapping TEXT, OBJECT_STORAGE_URI, and OBJECT_STORAGE_TUPLE output types
compartment_id = "ocid1.compartment"
instance_id = "ocid1.instance"

mock_client = MagicMock()
mock_get_client.return_value = mock_client

# First page with TEXT output content summary
# First page: TEXT output content
resp_page_1 = create_autospec(oci.response.Response)

summary_text = MagicMock()
setattr(summary_text, "instance_agent_command_id", "cmd-text-1")
setattr(summary_text, "instance_id", instance_id)
Expand All @@ -173,20 +181,22 @@ async def test_list_instance_agent_commands_pagination_and_limit_and_output_type
setattr(summary_text, "time_updated", "2024-01-01T00:00:05Z")
setattr(summary_text, "sequence_number", 10)
setattr(summary_text, "display_name", "text-name")
# Attach TEXT content

content_text = MagicMock()
content_text.output_type = "TEXT"
content_text.exit_code = 0
content_text.message = "ok"
content_text.text = "hello"
content_text.text_sha256 = "abc"
setattr(summary_text, "content", content_text)

resp_page_1.data = [summary_text]
resp_page_1.has_next_page = True
resp_page_1.next_page = "token-1"

# Second page with OBJECT_STORAGE_URI and OBJECT_STORAGE_TUPLE
# Second page: OBJECT_STORAGE_URI and OBJECT_STORAGE_TUPLE output
resp_page_2 = create_autospec(oci.response.Response)

summary_uri = MagicMock()
setattr(summary_uri, "instance_agent_command_id", "cmd-uri-2")
setattr(summary_uri, "instance_id", instance_id)
Expand All @@ -196,6 +206,7 @@ async def test_list_instance_agent_commands_pagination_and_limit_and_output_type
setattr(summary_uri, "time_updated", "2024-01-01T00:00:20Z")
setattr(summary_uri, "sequence_number", 11)
setattr(summary_uri, "display_name", "uri-name")

content_uri = MagicMock()
content_uri.output_type = "OBJECT_STORAGE_URI"
content_uri.exit_code = 1
Expand All @@ -222,15 +233,19 @@ async def test_list_instance_agent_commands_pagination_and_limit_and_output_type
setattr(summary_tuple, "content", content_tuple)

resp_page_2.data = [summary_uri, summary_tuple]
resp_page_2.has_next_page = False
resp_page_2.next_page = None

# Deliberately advertise another page. The overall limit should
# prevent a third SDK request after enough results have been collected.
resp_page_2.has_next_page = True
resp_page_2.next_page = "token-3"

mock_client.list_instance_agent_command_executions.side_effect = [
resp_page_1,
resp_page_2,
]

# Set limit=2 to stop after two items though there are three across pages
# There are three available items across the two mocked pages,
# but the caller requests only two.
limit = 2
async with Client(mcp) as client:
payload = {
Expand All @@ -239,27 +254,102 @@ async def test_list_instance_agent_commands_pagination_and_limit_and_output_type
"limit": limit,
}
result = (
await client.call_tool("list_instance_agent_command_executions", payload)
await client.call_tool(
"list_instance_agent_command_executions",
payload,
)
).structured_content["result"]

# Verify pagination and mapping of content subtypes
assert len(result) == 3
# First is TEXT
# The overall result limit is respected.
assert len(result) == 2

# First result: TEXT
assert result[0]["instance_agent_command_id"] == "cmd-text-1"
assert result[0]["content"]["output_type"] == "TEXT"
assert result[0]["content"]["text"] == "hello"
# Second is OBJECT_STORAGE_URI

# Second result: OBJECT_STORAGE_URI
assert result[1]["instance_agent_command_id"] == "cmd-uri-2"
assert result[1]["content"]["output_type"] == "OBJECT_STORAGE_URI"
assert result[1]["content"]["output_uri"] == "https://objectstorage.example.com/n/bkt/o/out"

# Ensure pagination called with correct page tokens
first_kwargs = mock_client.list_instance_agent_command_executions.call_args_list[0].kwargs
second_kwargs = mock_client.list_instance_agent_command_executions.call_args_list[1].kwargs
assert first_kwargs["page"] is None
assert first_kwargs["limit"] == limit
assert second_kwargs["page"] == "token-1"
assert second_kwargs["limit"] == limit
assert (
result[1]["content"]["output_uri"]
== "https://objectstorage.example.com/n/bkt/o/out"
)

# Verify the page tokens and requested limits.
calls = mock_client.list_instance_agent_command_executions.call_args_list

assert len(calls) == 2
assert calls[0].kwargs["page"] is None
assert calls[0].kwargs["limit"] == limit

assert calls[1].kwargs["page"] == "token-1"
assert calls[1].kwargs["limit"] == 1

# The requested limit decreases as results accumulate.
assert [call.kwargs["limit"] for call in calls] == [2, 1]

# No third request is made, despite the second response having
# has_next_page=True and next_page="token-3".
assert mock_client.list_instance_agent_command_executions.call_count == 2

@pytest.mark.asyncio
@patch(
"oracle.oci_compute_instance_agent_mcp_server.server."
"get_compute_instance_agent_client"
)
async def test_list_instance_agent_commands_continues_after_empty_page(
self, mock_get_client
):
mock_client = MagicMock()
mock_get_client.return_value = mock_client

empty_page = create_autospec(oci.response.Response)
empty_page.data = []
empty_page.has_next_page = True
empty_page.next_page = "token-2"

next_page = create_autospec(oci.response.Response)
next_page.data = [
InstanceAgentCommandExecutionSummary(
instance_agent_command_id="cmd-1",
instance_id="instance-1",
delivery_state="VISIBLE",
lifecycle_state="SUCCEEDED",
time_created="2024-01-01T00:00:00Z",
time_updated="2024-01-01T00:00:05Z",
sequence_number=1,
)
]
next_page.has_next_page = False
next_page.next_page = None

mock_client.list_instance_agent_command_executions.side_effect = [
empty_page,
next_page,
]

async with Client(mcp) as client:
result = (
await client.call_tool(
"list_instance_agent_command_executions",
{
"compartment_id": "compartment-1",
"instance_id": "instance-1",
},
)
).structured_content["result"]

assert [item["instance_agent_command_id"] for item in result] == [
"cmd-1"
]

calls = mock_client.list_instance_agent_command_executions.call_args_list
assert len(calls) == 2
assert calls[0].kwargs["page"] is None
assert calls[1].kwargs["page"] == "token-2"
assert calls[0].kwargs["limit"] is None
assert calls[1].kwargs["limit"] is None

@pytest.mark.asyncio
@patch("oracle.oci_compute_instance_agent_mcp_server.server.get_compute_instance_agent_client")
Expand Down
5 changes: 5 additions & 0 deletions src/oci-compute-mcp-server/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@

## Unreleased

### Fixed

- Fixed pagination limits in list tools to ensure results respect the requested maximum across API pages.

### Added

- `launch_instance` can now use an existing boot volume as its source by accepting `boot_volume_id`.

### Fixed

- `launch_instance` rejects empty `boot_volume_id` values instead of launching from the default image.
- Enforce the total result limit across paginated Compute MCP tool responses.

### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,11 @@ def list_instances(
next_page: str = None

while has_next_page and (limit is None or len(instances) < limit):
remaining = None if limit is None else limit - len(instances)
kwargs = {
"compartment_id": compartment_id,
"page": next_page,
"limit": limit,
"limit": remaining,
}

if lifecycle_state:
Expand All @@ -149,8 +150,10 @@ def list_instances(
data: list[oci.core.models.Instance] = response.data
for d in data:
instance = map_instance(d)
instances.append(instance)

if limit is None or len(instances) < limit:
instances.append(instance)
else:
break
logger.info(f"Found {len(instances)} Instances")
return instances

Expand Down Expand Up @@ -341,10 +344,11 @@ def list_images(
next_page: str = None

while has_next_page and (limit is None or len(images) < limit):
remaining = None if limit is None else limit - len(images)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from @dustin-sale via Codex.

[P2] Commit regressions for the pagination boundaries

Evidence: the PR changes 18 loops but changes tests only in Identity. Compute's tool tests contain no limit assertions. Supplemental review probes reproduce the original five-results-for-limit=3 behavior on the base revision and pass on this head, including filtered-image pagination.

Impact: high line coverage currently allows the original total-cap defect to pass undetected, leaving these fixes vulnerable to regression.

Requested change: add package-local regression coverage for the affected tools using short pages followed by continuation, empty pages with continuation, remaining request allowances, exact-cap stopping, and unlimited traversal. Include a filtered-out image page followed by matching images and verify the emitted match cap. The regressions should fail on the original implementation.

kwargs = {
"compartment_id": compartment_id,
"page": next_page,
"limit": limit,
"limit": remaining,
}

response = client.list_images(**kwargs)
Expand All @@ -357,7 +361,10 @@ def list_images(

for d in data:
image = map_image(d)
images.append(image)
if limit is None or len(images) < limit:
images.append(image)
else:
break

logger.info(f"Found {len(images)} Images")
return images
Expand Down Expand Up @@ -434,10 +441,11 @@ def list_vnic_attachments(
next_page: str = None

while has_next_page and (limit is None or len(vnic_attachments) < limit):
remaining = None if limit is None else limit - len(vnic_attachments)
kwargs = {
"compartment_id": compartment_id,
"page": next_page,
"limit": limit,
"limit": remaining,
}

if instance_id:
Expand All @@ -450,8 +458,10 @@ def list_vnic_attachments(
data: list[oci.core.models.VnicAttachment] = response.data

for d in data:
vnic_attachments.append(map_vnic_attachment(d))

if limit is None or len(vnic_attachments) < limit:
vnic_attachments.append(map_vnic_attachment(d))
else:
break
logger.info(f"Found {len(vnic_attachments)} Vnic Attachments")
return vnic_attachments

Expand Down
Loading
Loading