fix(engine): address Codex review (auth token bootstrap, tunnel recheck, ssh)

- CreateAutomationToken now bootstraps the pveum role/user/token over the
  authenticated SSH session (a fresh host has no API token yet); ProxmoxClient
  is read-only in A.
- ConfigureWireguard re-verifies the handshake on the idempotent replay path,
  never advancing over a dead tunnel.
- PhpseclibRemoteShell treats a missing SSH exit status as failure (255).
- connectWithKey verifies the pinned host key fingerprint on later logins.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-25 10:20:59 +02:00
parent 36a564d5c8
commit 65717bd3bd
12 changed files with 105 additions and 128 deletions

View File

@ -24,33 +24,34 @@ class ConfigureWireguard extends HostStep
public function execute(ProvisioningRun $run): StepResult
{
$host = $this->host($run);
// Idempotent: tunnel already provisioned and peer registered.
if (filled($host->wg_ip) && $this->hasResource($run, 'wg_peer')) {
return StepResult::advance();
}
$alreadyProvisioned = filled($host->wg_ip) && $this->hasResource($run, 'wg_peer');
$this->keyLogin($this->shell, $host);
$this->shell->run('export DEBIAN_FRONTEND=noninteractive; apt-get install -y wireguard');
$this->shell->run('test -f /etc/wireguard/privatekey || (umask 077; wg genkey > /etc/wireguard/privatekey)');
$publicKey = trim($this->shell->run('wg pubkey < /etc/wireguard/privatekey')->stdout);
if (blank($publicKey)) {
return StepResult::retry(20, 'could not read host WireGuard public key');
if (! $alreadyProvisioned) {
$this->shell->run('export DEBIAN_FRONTEND=noninteractive; apt-get install -y wireguard');
$this->shell->run('test -f /etc/wireguard/privatekey || (umask 077; wg genkey > /etc/wireguard/privatekey)');
$publicKey = trim($this->shell->run('wg pubkey < /etc/wireguard/privatekey')->stdout);
if (blank($publicKey)) {
return StepResult::retry(20, 'could not read host WireGuard public key');
}
$wgIp = $host->wg_ip ?: $this->hub->allocateIp();
$privateKey = trim($this->shell->run('cat /etc/wireguard/privatekey')->stdout);
$this->shell->putFile('/etc/wireguard/wg0.conf', $this->renderConfig($wgIp, $privateKey));
$this->shell->run('systemctl enable --now wg-quick@wg0 || wg-quick up wg0');
$this->hub->addPeer($publicKey, $wgIp);
// Persist external identity BEFORE verifying/advancing (crash-safe).
$host->update(['wg_ip' => $wgIp, 'wg_pubkey' => $publicKey]);
$this->recordResource($run, $host, 'wg_peer', $publicKey);
}
$wgIp = $host->wg_ip ?: $this->hub->allocateIp();
$privateKey = trim($this->shell->run('cat /etc/wireguard/privatekey')->stdout);
$this->shell->putFile('/etc/wireguard/wg0.conf', $this->renderConfig($wgIp, $privateKey));
$this->shell->run('systemctl enable --now wg-quick@wg0 || wg-quick up wg0');
$this->hub->addPeer($publicKey, $wgIp);
// Persist external identity BEFORE verifying/advancing (crash-safe).
$host->update(['wg_ip' => $wgIp, 'wg_pubkey' => $publicKey]);
$this->recordResource($run, $host, 'wg_peer', $publicKey);
// Always verify the tunnel is up — including the idempotent replay path,
// so we never advance onto Proxmox API calls over a dead tunnel.
$hubIp = (string) config('provisioning.wireguard.hub_ip', '10.66.0.1');
if (! $this->shell->run('ping -c1 -W2 '.escapeshellarg($hubIp))->ok()) {
return StepResult::retry(15, 'WireGuard handshake not up yet');

View File

@ -4,16 +4,18 @@ namespace App\Provisioning\Steps\Host;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Proxmox\ProxmoxClient;
use App\Services\Ssh\RemoteShell;
/**
* Creates the automation role + user + API token on the host and stores the
* (encrypted) token reference. The secret is shown only at creation, so the
* token id is persisted before advancing.
* Creates the automation role + user + API token on the host via `pveum` over
* SSH. A fresh host has no API token yet, so this bootstrap must run through the
* authenticated root SSH session, not the (token-less) REST API. The secret is
* shown only once, so the token id is persisted before advancing; a re-run
* deletes and re-mints the token to stay idempotent after a crash.
*/
class CreateAutomationToken extends HostStep
{
public function __construct(private ProxmoxClient $pve) {}
public function __construct(private RemoteShell $shell) {}
public function key(): string
{
@ -29,23 +31,39 @@ class CreateAutomationToken extends HostStep
return StepResult::advance();
}
$client = $this->pve->forHost($host);
$client->createRole(
config('provisioning.proxmox.role_id'),
config('provisioning.proxmox.role_privs'),
$role = (string) config('provisioning.proxmox.role_id');
$privs = (string) config('provisioning.proxmox.role_privs');
$user = (string) config('provisioning.proxmox.user');
$tokenName = (string) config('provisioning.proxmox.token_name');
$this->keyLogin($this->shell, $host);
// Role / user / ACL are idempotent (ignore "already exists").
$this->shell->run('pveum role add '.escapeshellarg($role).' -privs '.escapeshellarg($privs).' || true');
$this->shell->run('pveum user add '.escapeshellarg($user).' || true');
$this->shell->run('pveum acl modify / -user '.escapeshellarg($user).' -role '.escapeshellarg($role).' || true');
// Drop any half-created token from a prior crashed attempt, then mint fresh.
$this->shell->run('pveum user token remove '.escapeshellarg($user).' '.escapeshellarg($tokenName).' || true');
$result = $this->shell->run(
'pveum user token add '.escapeshellarg($user).' '.escapeshellarg($tokenName).' -privsep 0 --output-format json'
);
$token = $client->createUserAndToken(
config('provisioning.proxmox.user'),
config('provisioning.proxmox.role_id'),
);
if (! $result->ok()) {
return StepResult::retry(20, 'Proxmox token creation failed');
}
if (blank($token['secret'] ?? null)) {
$data = json_decode($result->stdout, true);
$secret = $data['value'] ?? null;
if (blank($secret)) {
return StepResult::retry(20, 'Proxmox token secret was empty');
}
$host->update(['api_token_ref' => $token['token_id'].'='.$token['secret']]);
$this->recordResource($run, $host, 'pve_token', $token['token_id']);
$tokenId = $data['full-tokenid'] ?? "{$user}!{$tokenName}";
$host->update(['api_token_ref' => $tokenId.'='.$secret]);
$this->recordResource($run, $host, 'pve_token', $tokenId);
return StepResult::advance();
}

View File

@ -34,7 +34,12 @@ abstract class HostStep implements ProvisioningStep
protected function keyLogin(RemoteShell $shell, Host $host): void
{
$shell->connectWithKey($host->public_ip, 'root', (string) config('provisioning.ssh.private_key'));
$shell->connectWithKey(
$host->public_ip,
'root',
(string) config('provisioning.ssh.private_key'),
$host->ssh_host_key, // pinned during EstablishSshTrust
);
}
protected function recordResource(ProvisioningRun $run, Host $host, string $kind, string $externalId): void

View File

@ -23,14 +23,6 @@ class FakeProxmoxClient implements ProxmoxClient
['storage' => 'local-lvm', 'type' => 'lvmthin', 'total' => 1099511627776], // 1 TiB
];
/** @var array<int, string> */
public array $createdRoles = [];
/** @var array{token_id: string, secret: string}|null */
public ?array $createdToken = null;
public int $tokenCalls = 0;
public function forHost(Host $host): static
{
$this->host = $host;
@ -52,19 +44,4 @@ class FakeProxmoxClient implements ProxmoxClient
{
return $this->storage;
}
public function createRole(string $roleId, string $privs): void
{
$this->createdRoles[] = $roleId;
}
public function createUserAndToken(string $user, string $roleId): array
{
$this->tokenCalls++;
return $this->createdToken = [
'token_id' => $user.'!clupilot',
'secret' => 'fake-secret-'.substr(md5($user.$roleId), 0, 12),
];
}
}

View File

@ -5,12 +5,13 @@ namespace App\Services\Proxmox;
use App\Models\Host;
use Illuminate\Http\Client\PendingRequest;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Str;
/**
* Real Proxmox VE REST client. Token auth over the WireGuard tunnel; the PVE
* certificate is self-signed so TLS verification is disabled (traffic already
* runs inside the encrypted tunnel). Not unit-tested (live I/O).
* Real Proxmox VE REST client (read-only in A: capacity + reachability). Token
* auth over the WireGuard tunnel; the PVE certificate is self-signed so TLS
* verification is disabled (traffic already runs inside the encrypted tunnel).
* Not unit-tested (live I/O). The automation token itself is bootstrapped via
* SSH `pveum` (see CreateAutomationToken), not here.
*/
class HttpProxmoxClient implements ProxmoxClient
{
@ -48,44 +49,4 @@ class HttpProxmoxClient implements ProxmoxClient
{
return $this->http()->get("/nodes/{$node}/storage")->throw()->json('data', []);
}
public function createRole(string $roleId, string $privs): void
{
$response = $this->http()->asForm()->post('/access/roles', [
'roleid' => $roleId,
'privs' => $privs,
]);
// Idempotent: an existing role is not an error.
if ($response->failed() && ! str_contains($response->body(), 'already exists')) {
$response->throw();
}
}
public function createUserAndToken(string $user, string $roleId): array
{
// Ensure the user exists (ignore "already exists"), grant the role, mint a token.
$this->http()->asForm()->post('/access/users', [
'userid' => $user,
'password' => Str::random(40),
]);
$this->http()->asForm()->put('/access/acl', [
'path' => '/',
'users' => $user,
'roles' => $roleId,
])->throw();
$tokenName = config('provisioning.proxmox.token_name', 'clupilot');
$data = $this->http()->asForm()
->post("/access/users/{$user}/token/{$tokenName}", ['privsep' => 0])
->throw()
->json('data', []);
return [
'token_id' => $data['full-tokenid'] ?? "{$user}!{$tokenName}",
'secret' => $data['value'] ?? '',
];
}
}

View File

@ -6,8 +6,9 @@ use App\Models\Host;
/**
* Proxmox VE REST client, addressed per host over its wg_ip with a token.
* A v1.0 reads capacity and provisions the automation token; Subsystem B
* extends it with VM lifecycle methods (clone, cloud-init, guestExec ).
* A v1.0 uses it read-only (capacity + reachability); the automation token is
* bootstrapped over SSH (pveum). Subsystem B extends it with VM lifecycle
* methods (clone, cloud-init, guestExec ).
*/
interface ProxmoxClient
{
@ -22,9 +23,4 @@ interface ProxmoxClient
/** @return array<int, array<string, mixed>> */
public function nodeStorage(string $node): array;
public function createRole(string $roleId, string $privs): void;
/** @return array{token_id: string, secret: string} */
public function createUserAndToken(string $user, string $roleId): array;
}

View File

@ -53,7 +53,7 @@ class FakeRemoteShell implements RemoteShell
$this->connections[] = ['password', $host, $user];
}
public function connectWithKey(string $host, string $user, string $privateKey): void
public function connectWithKey(string $host, string $user, string $privateKey, ?string $expectedFingerprint = null): void
{
if ($this->failConnect) {
throw new \RuntimeException("connection refused: {$host}");

View File

@ -26,7 +26,7 @@ class PhpseclibRemoteShell implements RemoteShell
$this->ssh = $ssh;
}
public function connectWithKey(string $host, string $user, string $privateKey): void
public function connectWithKey(string $host, string $user, string $privateKey, ?string $expectedFingerprint = null): void
{
$key = PublicKeyLoader::load($privateKey);
$ssh = new SSH2($host);
@ -36,6 +36,12 @@ class PhpseclibRemoteShell implements RemoteShell
}
$this->ssh = $ssh;
// Host-key pinning: reject a server whose key changed since onboarding.
if (filled($expectedFingerprint) && $this->hostKeyFingerprint() !== $expectedFingerprint) {
$this->ssh = null;
throw new RuntimeException("SSH host key mismatch for {$host} (possible interception or re-provision).");
}
}
public function run(string $command): CommandResult
@ -43,7 +49,8 @@ class PhpseclibRemoteShell implements RemoteShell
$stdout = (string) $this->ssh()->exec($command);
$exit = $this->ssh()->getExitStatus();
return new CommandResult(is_int($exit) ? $exit : 0, $stdout, '');
// No exit status = interrupted/closed channel → treat as failure, never success.
return new CommandResult(is_int($exit) ? $exit : 255, $stdout, '');
}
public function putFile(string $remotePath, string $contents): void

View File

@ -10,7 +10,12 @@ interface RemoteShell
{
public function connectWithPassword(string $host, string $user, string $password): void;
public function connectWithKey(string $host, string $user, string $privateKey): void;
/**
* Connect with a private key. When $expectedFingerprint is non-empty the
* server host key must match it, otherwise the connection is rejected
* (host-key pinning).
*/
public function connectWithKey(string $host, string $user, string $privateKey, ?string $expectedFingerprint = null): void;
public function run(string $command): CommandResult;

View File

@ -12,6 +12,9 @@ it('drives a fresh host all the way to active (mocked)', function () {
$s = fakeServices();
$s['shell']->script('wg pubkey', CommandResult::success('HOSTPUBKEY0000='));
$s['shell']->script('uname -r', CommandResult::success('6.8.12-4-pve'));
$s['shell']->script('pveum user token add', CommandResult::success(
json_encode(['full-tokenid' => 'automation@pve!clupilot', 'value' => 'tok-secret-123'])
));
$host = app(StartHostOnboarding::class)->run([
'name' => 'pve-fsn-9',
@ -53,6 +56,9 @@ it('does not duplicate external resources when a step re-runs after a crash', fu
$s = fakeServices();
$s['shell']->script('wg pubkey', CommandResult::success('HOSTPUBKEY0000='));
$s['shell']->script('uname -r', CommandResult::success('6.8.12-4-pve'));
$s['shell']->script('pveum user token add', CommandResult::success(
json_encode(['full-tokenid' => 'automation@pve!clupilot', 'value' => 'tok-secret-123'])
));
$host = app(StartHostOnboarding::class)->run([
'name' => 'pve-fsn-10',

View File

@ -180,20 +180,25 @@ it('fails when the host does not return before the reboot deadline', function ()
// --- CreateAutomationToken ---
it('creates and persists the automation token exactly once', function () {
it('creates and persists the automation token via pveum exactly once', function () {
$s = fakeServices();
$s['shell']->script('pveum user token add', CommandResult::success(
json_encode(['full-tokenid' => 'automation@pve!clupilot', 'value' => 'tok-secret-123'])
));
$host = Host::factory()->create();
$run = hostRun($host);
expect(app(CreateAutomationToken::class)->execute($run)->type)->toBe('advance');
$host->refresh();
expect($host->api_token_ref)->toContain('automation@pve')
->and($host->api_token_ref)->toContain('fake-secret')
->and($s['pve']->tokenCalls)->toBe(1);
expect($host->api_token_ref)->toContain('automation@pve!clupilot')
->and($host->api_token_ref)->toContain('tok-secret-123');
// Re-run: idempotent short-circuit, no second token call.
$tokenCalls = fn () => count(array_filter($s['shell']->recorded(), fn ($c) => str_contains($c, 'pveum user token add')));
expect($tokenCalls())->toBe(1);
// Re-run: idempotent short-circuit, no second token creation.
app(CreateAutomationToken::class)->execute($run->fresh());
expect($s['pve']->tokenCalls)->toBe(1)
expect($tokenCalls())->toBe(1)
->and(RunResource::where('run_id', $run->id)->where('kind', 'pve_token')->count())->toBe(1);
});

View File

@ -34,15 +34,11 @@ it('allocates ips and tracks peers (FakeWireguardHub)', function () {
expect($hub->peers())->toBe([]);
});
it('reports capacity and mints a token for a host (FakeProxmoxClient)', function () {
it('reports node capacity for a host (FakeProxmoxClient)', function () {
$host = Host::factory()->create(['wg_ip' => '10.66.0.5']);
$client = (new FakeProxmoxClient)->forHost($host);
expect($client->listNodes())->not->toBeEmpty()
->and($client->nodeStatus('pve')['memory']['total'])->toBeGreaterThan(0)
->and($client->nodeStorage('pve')[0]['total'])->toBeGreaterThan(0);
$token = $client->createUserAndToken('automation@pve', 'CluPilotAutomation');
expect($token)->toHaveKeys(['token_id', 'secret'])
->and($client->tokenCalls)->toBe(1);
});