fix: Remove client-specified port from docker_start/auto_start
Ports are now always allocated from the pool (8192-8199) automatically. This prevents session collisions where different agents would specify the same port and interfere with each other. Clients can't accidentally (or intentionally) override the port allocation — the pool manager handles all assignments.
This commit is contained in:
parent
458d4fb35b
commit
d1f8779f05
@ -510,7 +510,6 @@ class DockerMixin(MCPMixin):
|
|||||||
async def docker_start(
|
async def docker_start(
|
||||||
self,
|
self,
|
||||||
binary_path: str,
|
binary_path: str,
|
||||||
port: Optional[int] = None,
|
|
||||||
memory: str = "2G",
|
memory: str = "2G",
|
||||||
name: Optional[str] = None,
|
name: Optional[str] = None,
|
||||||
ctx: Optional[Context] = None,
|
ctx: Optional[Context] = None,
|
||||||
@ -519,15 +518,14 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
This creates a new Ghidra instance in Docker with the GhydraMCP
|
This creates a new Ghidra instance in Docker with the GhydraMCP
|
||||||
plugin pre-installed. The binary will be imported and analyzed,
|
plugin pre-installed. The binary will be imported and analyzed,
|
||||||
then the HTTP API will be available on the specified port.
|
then the HTTP API will be available.
|
||||||
|
|
||||||
If no port is specified, one will be automatically allocated from
|
Ports are automatically allocated from the pool (8192-8199) to
|
||||||
the pool (8192-8199). Container names are auto-generated with the
|
prevent conflicts between concurrent sessions. Container names
|
||||||
session ID to ensure uniqueness across processes.
|
are auto-generated with the session ID to ensure uniqueness.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
binary_path: Path to the binary file to analyze
|
binary_path: Path to the binary file to analyze
|
||||||
port: Port to expose the HTTP API (auto-allocated if not specified)
|
|
||||||
memory: Max JVM heap memory (default: 2G)
|
memory: Max JVM heap memory (default: 2G)
|
||||||
name: Container name (auto-generated if not specified)
|
name: Container name (auto-generated if not specified)
|
||||||
|
|
||||||
@ -542,16 +540,13 @@ class DockerMixin(MCPMixin):
|
|||||||
if not binary_file.exists():
|
if not binary_file.exists():
|
||||||
return {"error": f"Binary not found: {binary_path}"}
|
return {"error": f"Binary not found: {binary_path}"}
|
||||||
|
|
||||||
# Allocate port from pool if not specified
|
# Always allocate from pool to prevent conflicts between sessions
|
||||||
allocated_port = False
|
|
||||||
if port is None:
|
|
||||||
port = self._port_pool.allocate(self.session_id)
|
port = self._port_pool.allocate(self.session_id)
|
||||||
if port is None:
|
if port is None:
|
||||||
return {
|
return {
|
||||||
"error": "Port pool exhausted (8192-8199). Stop some containers first.",
|
"error": "Port pool exhausted (8192-8199). Stop some containers first.",
|
||||||
"allocated_ports": self._port_pool.get_allocated_ports(),
|
"allocated_ports": self._port_pool.get_allocated_ports(),
|
||||||
}
|
}
|
||||||
allocated_port = True
|
|
||||||
|
|
||||||
# Generate container name if not specified
|
# Generate container name if not specified
|
||||||
if name is None:
|
if name is None:
|
||||||
@ -566,7 +561,6 @@ class DockerMixin(MCPMixin):
|
|||||||
["ps", "-a", "-q", "-f", f"name=^{name}$"], check=False
|
["ps", "-a", "-q", "-f", f"name=^{name}$"], check=False
|
||||||
)
|
)
|
||||||
if check_result.stdout.strip():
|
if check_result.stdout.strip():
|
||||||
if allocated_port:
|
|
||||||
self._port_pool.release(port)
|
self._port_pool.release(port)
|
||||||
return {
|
return {
|
||||||
"error": f"Container '{name}' already exists. Stop it first with docker_stop."
|
"error": f"Container '{name}' already exists. Stop it first with docker_stop."
|
||||||
@ -577,7 +571,6 @@ class DockerMixin(MCPMixin):
|
|||||||
["ps", "-q", "-f", f"publish={port}"], check=False
|
["ps", "-q", "-f", f"publish={port}"], check=False
|
||||||
)
|
)
|
||||||
if port_check.stdout.strip():
|
if port_check.stdout.strip():
|
||||||
if allocated_port:
|
|
||||||
self._port_pool.release(port)
|
self._port_pool.release(port)
|
||||||
return {
|
return {
|
||||||
"error": f"Port {port} is already in use by another container"
|
"error": f"Port {port} is already in use by another container"
|
||||||
@ -616,7 +609,6 @@ class DockerMixin(MCPMixin):
|
|||||||
"port": port,
|
"port": port,
|
||||||
"binary": str(binary_file),
|
"binary": str(binary_file),
|
||||||
"memory": memory,
|
"memory": memory,
|
||||||
"allocated_port": allocated_port,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@ -635,7 +627,6 @@ class DockerMixin(MCPMixin):
|
|||||||
}
|
}
|
||||||
|
|
||||||
except subprocess.CalledProcessError as e:
|
except subprocess.CalledProcessError as e:
|
||||||
if allocated_port:
|
|
||||||
self._port_pool.release(port)
|
self._port_pool.release(port)
|
||||||
return {"error": f"Failed to start container: {e.stderr or e.stdout}"}
|
return {"error": f"Failed to start container: {e.stderr or e.stdout}"}
|
||||||
|
|
||||||
@ -899,7 +890,6 @@ class DockerMixin(MCPMixin):
|
|||||||
async def docker_auto_start(
|
async def docker_auto_start(
|
||||||
self,
|
self,
|
||||||
binary_path: str,
|
binary_path: str,
|
||||||
port: Optional[int] = None,
|
|
||||||
wait: bool = True,
|
wait: bool = True,
|
||||||
timeout: float = 300.0,
|
timeout: float = 300.0,
|
||||||
ctx: Optional[Context] = None,
|
ctx: Optional[Context] = None,
|
||||||
@ -907,18 +897,16 @@ class DockerMixin(MCPMixin):
|
|||||||
"""Automatically start a Docker container with intelligent port allocation.
|
"""Automatically start a Docker container with intelligent port allocation.
|
||||||
|
|
||||||
This is the main entry point for automatic Docker management:
|
This is the main entry point for automatic Docker management:
|
||||||
1. Checks if a Ghidra instance is already running (on specified or any pooled port)
|
1. Checks if a Ghidra instance with the SAME binary is already running
|
||||||
2. If not, allocates a port from the pool and starts a new container
|
2. If not, allocates a port from the pool and starts a new container
|
||||||
3. Optionally waits for the container to become healthy
|
3. Optionally waits for the container to become healthy
|
||||||
4. Returns connection info for the instance
|
4. Returns connection info for the instance
|
||||||
|
|
||||||
When port is not specified, the system will:
|
Ports are auto-allocated from the pool (8192-8199) to prevent
|
||||||
- First check all pooled ports (8192-8199) for an existing healthy instance
|
conflicts between concurrent sessions.
|
||||||
- If none found, allocate a new port from the pool
|
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
binary_path: Path to the binary to analyze
|
binary_path: Path to the binary to analyze
|
||||||
port: Specific port for the HTTP API (auto-allocated if not specified)
|
|
||||||
wait: Wait for container to be ready (default: True)
|
wait: Wait for container to be ready (default: True)
|
||||||
timeout: Max wait time in seconds (default: 300)
|
timeout: Max wait time in seconds (default: 300)
|
||||||
|
|
||||||
@ -935,19 +923,6 @@ class DockerMixin(MCPMixin):
|
|||||||
return False
|
return False
|
||||||
return os.path.basename(health_program) == requested_name
|
return os.path.basename(health_program) == requested_name
|
||||||
|
|
||||||
# If port is specified, check that specific port
|
|
||||||
if port is not None:
|
|
||||||
health = await self.docker_health(port=port, ctx=ctx)
|
|
||||||
if health.get("healthy") and _is_same_binary(health.get("program", "")):
|
|
||||||
return {
|
|
||||||
"source": "existing",
|
|
||||||
"session_id": self.session_id,
|
|
||||||
"port": port,
|
|
||||||
"api_url": f"http://localhost:{port}/",
|
|
||||||
"program": health.get("program"),
|
|
||||||
"message": "Using existing Ghidra instance",
|
|
||||||
}
|
|
||||||
else:
|
|
||||||
# Check all pooled ports for an instance with the SAME binary
|
# Check all pooled ports for an instance with the SAME binary
|
||||||
for check_port in range(PORT_POOL_START, PORT_POOL_END + 1):
|
for check_port in range(PORT_POOL_START, PORT_POOL_END + 1):
|
||||||
health = await self.docker_health(port=check_port, timeout=1.0, ctx=ctx)
|
health = await self.docker_health(port=check_port, timeout=1.0, ctx=ctx)
|
||||||
@ -977,9 +952,9 @@ class DockerMixin(MCPMixin):
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
# Start a new container (port will be auto-allocated if not specified)
|
# Start a new container (port auto-allocated from pool)
|
||||||
start_result = await self.docker_start(
|
start_result = await self.docker_start(
|
||||||
binary_path=binary_path, port=port, ctx=ctx
|
binary_path=binary_path, ctx=ctx
|
||||||
)
|
)
|
||||||
|
|
||||||
if not start_result.get("success"):
|
if not start_result.get("success"):
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user