Codex/import artifact byte replication retry3#951
Conversation
lupuszr
force-pushed
the
codex/import-artifact-byte-replication-retry3
branch
from
9aa8629 to
379be8e
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| OPTIONAL { <${assertionUri}> <${ns}mdIntermediateHash> ?mdIntermediateHash } | ||
| }`, | ||
| ]); | ||
| const result = await this.store.query( |
There was a problem hiding this comment.
🔴 Bug: This lookup only checks whether the assertion has sourceFile/sourceFileHash linkage. It never verifies that assertionUri is actually a completed imported artifact (extractionStatus=completed, structural triples present, etc.), so any public+open assertion that references an existing urn:dkg:file:<hash> can now expose file-store bytes over the new protocol. Please reuse the same completed-import guard that /import-artifact/resolve applies before serving bytes.
|
|
||
| switch (response.status) { | ||
| case IMPORTED_ARTIFACT_BYTES_RESPONSE_STATUS.ALLOW: { | ||
| const bytes = Buffer.from(response.bytes); |
There was a problem hiding this comment.
🔴 Bug: We buffer and persist whatever payload the origin peer returns, and only enforce the caller's maxBytes later in the HTTP route. A buggy/malicious peer can therefore make us download and cache multi-MB content even when the caller requested a tiny read. Please reject oversized response.bytes before fileStore.put() (at least against the protocol cap, and ideally against the route's maxBytes).
| @@ -1325,7 +1515,7 @@ export async function handleAssertionRoutes(ctx: RequestContext): Promise<void> | |||
| } | |||
| return jsonResponse(res, 200, { | |||
| artifact, | |||
There was a problem hiding this comment.
🟡 Issue: artifact can still report canReadMarkdown: false here because resolveImportedArtifact() only checks the local cache, but this handler now succeeds after fetching from the origin peer. That leaves a 200 response with self-contradictory metadata and will also mislead callers that gate on /import-artifact/resolve. Consider updating the returned artifact/capability once the remote fetch path succeeds.
2 participants
Summary
/api/assertion/import-artifact/read-filefor source/original byte reads, while keeping/read-markdownmarkdown-onlyValidation
pnpm --dir packages/core exec vitest run test/import-artifact-bytes-proto.test.tspnpm --dir packages/agent exec vitest run test/dkg-agent-imported-artifact-bytes.test.ts --config vitest.unit.config.tspnpm --dir packages/cli exec vitest run test/import-artifact-read-file-routes.test.ts --config vitest.unit.config.tspnpm --dir packages/core exec tsc --noEmit --pretty false --tsBuildInfoFile /dev/nullpnpm --dir packages/agent exec tsc --noEmit --pretty false --tsBuildInfoFile /dev/nullpnpm --dir packages/cli exec tsc --noEmit --pretty false --tsBuildInfoFile /dev/nullgit diff --check