Skip to content

Commit 03afbaa

Browse files
nficanoclaude
nficano
and
claude
authored
fix: address 17 open issues across auth, isolation, idempotency, and docs (#52)
Security & isolation: - BearerAuth ignores client-supplied principal; only the server-side token mapping is authoritative (#34). - Resume replay is scoped to the calling session via EventLog::replayAfterForSession (#44). - Subscriptions with no session_id filter default to the calling session instead of fanning out across sessions (#45). - Artifact fetch/release reject cross-session artifact ids with PERMISSION_DENIED (#46). - LeaseManager records the granting session; tool.invoke rejects leases granted to another session (#47). Correctness: - Idempotent retries now replay the original tool.result/tool.error instead of returning a bare Ack (#35). - ResultChunkAssembler enforces sequence contiguity, terminal-chunk delivery, and duplicate consistency (#36). - CostBudget tolerates small float costs (scientific notation), rejects NaN/Inf, and rejects over-precise budget patterns (#37). - ArtifactPut validates the supplied sha256 digest against payload bytes; client putArtifact() accepts and surfaces digest errors (#38). - ARCPClient ping/putArtifact map Nack responses through ErrorMapper consistently with the other helpers (#39). - StdioTransport drains and decodes a final unterminated frame on EOF instead of dropping it (#48). - InMemoryCredentialStore now reports supportsDurableRevocation() as false; runtime correctly refuses to pair it with a CredentialProvisioner (#49). - JobManager retains terminal jobs for a bounded retention window so session.list_jobs can surface recent history (#50). Documentation: - README event type names match the actual catalog (#40). - Auth guide notes server-authoritative bearer principal (#41). - @throws docs added to ARCPClient, JobContext, ARCPRuntime, and Transport public APIs (#42). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 09d40da commit 03afbaa

35 files changed

Lines changed: 1141 additions & 60 deletions

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ printf("resolved value = %s\n", json_encode($result->value));
172172

173173
### Consuming events
174174

175-
Iterate the ordered event stream — `log`, `thought`, `tool_call`, `tool_result`, `status`, `metric`, `artifact_ref`, `progress`, `result_chunk` — and optionally acknowledge progress so the runtime can release buffered events early.
175+
Iterate the ordered event stream — `log`, `metric`, `event.emit`, `tool.invoke`, `tool.result`, `job.progress`, `job.result_chunk`, `artifact.ref` — and optionally acknowledge progress so the runtime can release buffered events early.
176176

177177
```php
178178
use Arcp\Envelope\Envelope;

‎docs/guides/auth.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ $runtime = new ARCPRuntime(
3030
);
3131
```
3232

33+
`BearerAuth` always resolves the principal from the token → principal
34+
map. The `principal` field a client may set in `PeerInfo` is ignored on
35+
the runtime side — only the server-side mapping is authoritative.
36+
3337
## Custom verifier
3438

3539
Implement `Arcp\Auth\AuthScheme`:
@@ -54,8 +58,9 @@ final class HeaderAuth implements AuthScheme
5458
## Where the principal lives
5559

5660
After handshake, the runtime stores the resolved principal on
57-
`Session::$principal`. The client stores its own principal from
58-
`PeerInfo`.
61+
`Session::$principal` — server-authoritative, derived from the auth
62+
scheme, not from any client-supplied `PeerInfo`. The client stores its
63+
own principal from `PeerInfo` purely for local logging.
5964

6065
## Sessions, resume, and auth
6166

‎src/Auth/BearerAuth.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public function verify(Auth $auth, PeerInfo $client): AuthResult
3636
if (!isset($this->tokens[$auth->token])) {
3737
return AuthResult::reject('invalid token');
3838
}
39-
$principal = $client->principal ?? $this->tokens[$auth->token];
40-
return AuthResult::accept($principal);
39+
// Always use the server-side principal mapped to the token; do not
40+
// trust the principal supplied in the untrusted PeerInfo block.
41+
return AuthResult::accept($this->tokens[$auth->token]);
4142
}
4243
}

‎src/Client/ARCPClient.php‎

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
use Arcp\Messages\Artifacts\ArtifactFetch;
3232
use Arcp\Messages\Artifacts\ArtifactPut;
3333
use Arcp\Messages\Artifacts\ArtifactRef;
34+
use Arcp\Messages\Artifacts\ArtifactRelease;
35+
use Arcp\Messages\Control\Ack;
3436
use Arcp\Messages\Control\Cancel;
3537
use Arcp\Messages\Control\Nack;
3638
use Arcp\Messages\Control\Ping;
@@ -141,6 +143,11 @@ public static function withConfig(ClientConfig $config): self
141143
/**
142144
* Send `session.open`, await `session.accepted`, and start the read-loop.
143145
*
146+
* @throws \Arcp\Errors\UnauthenticatedException when the runtime rejects credentials.
147+
* @throws \Arcp\Errors\UnimplementedException for capability mismatches.
148+
* @throws \Arcp\Errors\ARCPExceptionInterface for other handshake errors.
149+
* @throws \Arcp\Errors\TransportClosedException if the transport drops.
150+
*
144151
* @size-check-suppress public BC; mirrors RFC §8.3 session.open shape.
145152
*/
146153
public function open(
@@ -169,6 +176,14 @@ public function open(
169176
*
170177
* @param array<string, mixed> $arguments
171178
*
179+
* @throws \Arcp\Errors\ARCPExceptionInterface mapped from `tool.error`
180+
* or correlated `nack` (e.g. `PermissionDeniedException`,
181+
* `BudgetExhaustedException`, `NotFoundException`).
182+
* @throws \Arcp\Errors\DeadlineExceededException when `deadlineSeconds`
183+
* elapses before a terminal response arrives.
184+
* @throws \Arcp\Errors\CancelledException when `$cancellation` fires.
185+
* @throws InvalidArgumentException for unexpected response shapes.
186+
*
172187
* @size-check-suppress public BC; tool.invoke options are RFC §10 wire fields.
173188
*/
174189
public function invokeTool(
@@ -208,6 +223,11 @@ public function invokeTool(
208223
* @param array<string, mixed> $filter
209224
* @param \Closure(Envelope): void $onEvent Called with the unwrapped envelope.
210225
*
226+
* @throws \Arcp\Errors\PermissionDeniedException when the filter
227+
* crosses session-scope boundaries.
228+
* @throws \Arcp\Errors\ARCPExceptionInterface for other runtime errors.
229+
* @throws InvalidArgumentException for unexpected response shapes.
230+
*
211231
* @size-check-suppress public BC; subscribe is the RFC §13 entry-point.
212232
*/
213233
public function subscribe(
@@ -249,7 +269,13 @@ public function unsubscribe(SubscriptionId $id): void
249269
}
250270

251271
/**
272+
* Page through jobs visible to this session.
273+
*
252274
* @param array<string, mixed> $filter
275+
*
276+
* @throws \Arcp\Errors\ARCPExceptionInterface for runtime errors mapped
277+
* from a correlated `nack`.
278+
* @throws InvalidArgumentException for unexpected response shapes.
253279
*/
254280
public function listJobs(
255281
array $filter = [],
@@ -289,6 +315,13 @@ public function cancelJob(
289315
$this->session->transport->send($env);
290316
}
291317

318+
/**
319+
* Round-trip a ping/pong heartbeat.
320+
*
321+
* @throws \Arcp\Errors\ARCPExceptionInterface when the runtime
322+
* returns a Nack instead of a Pong.
323+
* @throws InvalidArgumentException for an unexpected response type.
324+
*/
292325
public function ping(?string $nonce = null, float $deadlineSeconds = 5.0): Pong
293326
{
294327
$id = MessageId::random();
@@ -299,29 +332,60 @@ public function ping(?string $nonce = null, float $deadlineSeconds = 5.0): Pong
299332
sessionId: $this->session->sessionId,
300333
);
301334
$this->session->transport->send($env);
302-
/** @var Pong $resp */
303335
$resp = $this->pending->awaitResponse($id, $deadlineSeconds);
336+
if ($resp instanceof Nack) {
337+
throw $this->errorMapper->raise($resp->error);
338+
}
339+
if (!$resp instanceof Pong) {
340+
throw new InvalidArgumentException('expected pong as ping response');
341+
}
304342
return $resp;
305343
}
306344

345+
/**
346+
* Upload an artifact and receive its server-issued reference.
347+
*
348+
* @param string|null $sha256 hex-encoded SHA-256 digest of `$bytes`.
349+
* When supplied, the runtime rejects the upload if the digest does
350+
* not match the decoded payload.
351+
*
352+
* @throws \Arcp\Errors\InvalidArgumentException on digest mismatch or
353+
* malformed payload.
354+
* @throws \Arcp\Errors\ARCPExceptionInterface on other runtime errors.
355+
*/
307356
public function putArtifact(
308357
string $mediaType,
309358
string $bytes,
310359
?int $retentionSeconds = null,
360+
?string $sha256 = null,
311361
): ArtifactRef {
312362
$id = MessageId::random();
313363
$env = new Envelope(
314364
id: $id,
315-
payload: new ArtifactPut($mediaType, base64_encode($bytes), $retentionSeconds),
365+
payload: new ArtifactPut($mediaType, base64_encode($bytes), $retentionSeconds, $sha256),
316366
timestamp: $this->clock->now(),
317367
sessionId: $this->session->sessionId,
318368
);
319369
$this->session->transport->send($env);
320-
/** @var ArtifactRef $resp */
321370
$resp = $this->pending->awaitResponse($id, 30.0);
371+
if ($resp instanceof Nack) {
372+
throw $this->errorMapper->raise($resp->error);
373+
}
374+
if (!$resp instanceof ArtifactRef) {
375+
throw new InvalidArgumentException('expected artifact.ref as put response');
376+
}
322377
return $resp;
323378
}
324379

380+
/**
381+
* Fetch the bytes of an artifact owned by this session.
382+
*
383+
* @throws \Arcp\Errors\PermissionDeniedException for cross-session ids.
384+
* @throws \Arcp\Errors\NotFoundException if the artifact is unknown
385+
* or has expired.
386+
* @throws \Arcp\Errors\ARCPExceptionInterface for other runtime errors.
387+
* @throws InvalidArgumentException for malformed payload or response.
388+
*/
325389
public function fetchArtifact(ArtifactId $artifactId): string
326390
{
327391
$id = MessageId::random();
@@ -345,6 +409,34 @@ public function fetchArtifact(ArtifactId $artifactId): string
345409
: throw new InvalidArgumentException('artifact data not base64');
346410
}
347411

412+
/**
413+
* Release an artifact owned by this session. Returns true if the
414+
* runtime confirmed deletion, false if it was already unknown.
415+
*
416+
* @throws \Arcp\Errors\PermissionDeniedException when the artifact
417+
* belongs to a different session.
418+
* @throws \Arcp\Errors\ARCPExceptionInterface on other runtime errors.
419+
*/
420+
public function releaseArtifact(ArtifactId $artifactId): bool
421+
{
422+
$id = MessageId::random();
423+
$env = new Envelope(
424+
id: $id,
425+
payload: new ArtifactRelease($artifactId),
426+
timestamp: $this->clock->now(),
427+
sessionId: $this->session->sessionId,
428+
);
429+
$this->session->transport->send($env);
430+
$resp = $this->pending->awaitResponse($id, 30.0);
431+
if ($resp instanceof Nack) {
432+
throw $this->errorMapper->raise($resp->error);
433+
}
434+
if (!$resp instanceof Ack) {
435+
throw new InvalidArgumentException('expected ack as artifact.release response');
436+
}
437+
return $resp->note === 'released';
438+
}
439+
348440
public function close(): void
349441
{
350442
if ($this->session->state === SessionState::Closed) {

‎src/Client/ResultChunkAssembler.php‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77
use Arcp\Errors\InvalidArgumentException;
88
use Arcp\Messages\Execution\ResultChunk;
99

10-
/** Collects `job.result_chunk` messages by result id and assembles final bytes. */
10+
/**
11+
* Collects `job.result_chunk` messages by result id and assembles final
12+
* bytes. Enforces sequence contiguity (0..N), terminal-chunk delivery,
13+
* and duplicate consistency so a truncated or out-of-order stream never
14+
* silently produces a corrupted result.
15+
*/
1116
final class ResultChunkAssembler
1217
{
1318
/** @var array<string, array<int, ResultChunk>> */
@@ -18,6 +23,13 @@ final class ResultChunkAssembler
1823

1924
public function push(ResultChunk $chunk): void
2025
{
26+
$existing = $this->chunks[$chunk->resultId][$chunk->chunkSeq] ?? null;
27+
if ($existing instanceof ResultChunk && !$this->sameChunkPayload($existing, $chunk)) {
28+
throw new InvalidArgumentException(
29+
'result_chunk duplicate with conflicting payload',
30+
['result_id' => $chunk->resultId, 'chunk_seq' => $chunk->chunkSeq],
31+
);
32+
}
2133
$this->chunks[$chunk->resultId][$chunk->chunkSeq] = $chunk;
2234
if (!$chunk->more) {
2335
$this->complete[$chunk->resultId] = true;
@@ -35,7 +47,14 @@ public function assemble(string $resultId): string
3547
if ($chunks === []) {
3648
throw new InvalidArgumentException('unknown result_id: ' . $resultId);
3749
}
50+
if (!isset($this->complete[$resultId])) {
51+
throw new InvalidArgumentException(
52+
'result_chunk stream incomplete: terminal chunk not yet received',
53+
['result_id' => $resultId],
54+
);
55+
}
3856
ksort($chunks);
57+
$this->assertContiguous($resultId, $chunks);
3958
$out = '';
4059
foreach ($chunks as $chunk) {
4160
$out .= $chunk->encoding === 'base64'
@@ -45,6 +64,36 @@ public function assemble(string $resultId): string
4564
return $out;
4665
}
4766

67+
/** @param array<int, ResultChunk> $chunks */
68+
private function assertContiguous(string $resultId, array $chunks): void
69+
{
70+
$expected = 0;
71+
$terminal = null;
72+
foreach ($chunks as $seq => $chunk) {
73+
if ($seq !== $expected) {
74+
throw new InvalidArgumentException(
75+
'result_chunk sequence not contiguous from 0',
76+
['result_id' => $resultId, 'expected_seq' => $expected, 'actual_seq' => $seq],
77+
);
78+
}
79+
++$expected;
80+
$terminal = $chunk;
81+
}
82+
if ($terminal instanceof ResultChunk && $terminal->more) {
83+
throw new InvalidArgumentException(
84+
'result_chunk highest sequence has more=true',
85+
['result_id' => $resultId],
86+
);
87+
}
88+
}
89+
90+
private function sameChunkPayload(ResultChunk $a, ResultChunk $b): bool
91+
{
92+
return $a->data === $b->data
93+
&& $a->encoding === $b->encoding
94+
&& $a->more === $b->more;
95+
}
96+
4897
private function decodeBase64(ResultChunk $chunk): string
4998
{
5099
$decoded = base64_decode($chunk->data, strict: true);

‎src/Internal/Runtime/ArtifactDispatcher.php‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,28 @@ public function put(Session $session, Envelope $env, ArtifactPut $msg): void
4040
);
4141
return;
4242
}
43+
if ($msg->sha256 !== null) {
44+
$normalized = strtolower(trim($msg->sha256));
45+
if (preg_match('/^[0-9a-f]{64}$/', $normalized) !== 1) {
46+
$this->lifecycle->nack(
47+
$session,
48+
$env,
49+
'INVALID_ARGUMENT',
50+
'artifact.put sha256 must be 64 lowercase hex chars',
51+
);
52+
return;
53+
}
54+
$computed = hash('sha256', $bytes);
55+
if (!hash_equals($computed, $normalized)) {
56+
$this->lifecycle->nack(
57+
$session,
58+
$env,
59+
'INVALID_ARGUMENT',
60+
'artifact.put sha256 does not match payload',
61+
);
62+
return;
63+
}
64+
}
4365
$ref = $this->runtime->artifacts->put(
4466
$session,
4567
new ArtifactBlob($msg->mediaType, $bytes, $msg->retentionSeconds),
@@ -50,21 +72,27 @@ public function put(Session $session, Envelope $env, ArtifactPut $msg): void
5072
public function fetch(Session $session, Envelope $env, ArtifactFetch $msg): void
5173
{
5274
try {
53-
$bytes = $this->runtime->artifacts->fetch($msg->artifactId);
75+
$bytes = $this->runtime->artifacts->fetch($msg->artifactId, $session);
76+
$mediaType = $this->runtime->artifacts->ref($msg->artifactId, $session)->mediaType;
5477
} catch (ARCPException $e) {
5578
$this->lifecycle->nack($session, $env, $e->code()->value, $e->getMessage());
5679
return;
5780
}
5881
$put = new ArtifactPut(
59-
mediaType: $this->runtime->artifacts->ref($msg->artifactId)->mediaType,
82+
mediaType: $mediaType,
6083
data: base64_encode($bytes),
6184
);
6285
$this->runtime->emit($session, $put, ['correlation_id' => $env->id]);
6386
}
6487

6588
public function release(Session $session, Envelope $env, ArtifactRelease $msg): void
6689
{
67-
$ok = $this->runtime->artifacts->release($msg->artifactId);
90+
try {
91+
$ok = $this->runtime->artifacts->release($msg->artifactId, $session);
92+
} catch (ARCPException $e) {
93+
$this->lifecycle->nack($session, $env, $e->code()->value, $e->getMessage());
94+
return;
95+
}
6896
$this->runtime->emit($session, new Ack($ok ? 'released' : 'unknown'), [
6997
'correlation_id' => $env->id,
7098
]);

0 commit comments

Comments
 (0)