fix: address PR 265 review findings#279
Open
bussyjd wants to merge 1 commit intofeat/autoresearch-integrationfrom
Open
fix: address PR 265 review findings#279bussyjd wants to merge 1 commit intofeat/autoresearch-integrationfrom
bussyjd wants to merge 1 commit intofeat/autoresearch-integrationfrom
Conversation
…erage Critical: - Fix path traversal in worker_api.py: validate experiment_id with regex - Add GGUF format guard in publish.py before ollama_create() - Run worker container as non-root user in Dockerfile Medium: - Replace manual provenance struct-to-map with JSON round-trip in sell.go - Fix weak test assertion in TestApproximateRequestPriceFromPerHour - Guard int(amount) cast in coordinate.py with try/except - Remove domain-specific default "val_bpb" from CRD metricName field - Guard maxTimeoutSeconds parse in monetize.py Low: - Add provenance propagation test for build_registration_doc - Fix doc type inconsistency in coordination-protocol.md - Assert all 6 Provenance fields in store_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses review findings from PR #265 (autoresearch marketplace foundations).
Critical fixes
worker_api.py: User-suppliedexperiment_idis now validated with^[a-zA-Z0-9_-]+$regex in both POST and GET handlers, preventing directory traversal attackspublish.py: Added explicit check beforeollama_create()— non-GGUF checkpoints now get a clear error with conversion instructions instead of a confusing Ollama failureworkeruser, reducing privilege escalation risk from arbitrary code executionMedium fixes
sell.go): Replaced 6 manualifblocks with a JSON marshal/unmarshal round-trip — new Provenance fields are now automatically includedpayment_test.go):TestApproximateRequestPriceFromPerHournow asserts exact expected values instead of just "not empty"int(amount)(coordinate.py): Wrapped in try/except to handle non-integer pricing strings gracefullyserviceoffer-crd.yaml): Removeddefault: "val_bpb"fromspec.provenance.metricName— too domain-specific for a generic CRD fieldmaxTimeoutSeconds(monetize.py): Wrapped parse in try/except for non-numeric valuesLow priority
build_registration_doccoordination-protocol.md(metadata fields are strings, not float/int)store_test.goTest plan
go build ./...passesgo test ./internal/inference/... ./internal/schemas/... ./cmd/obol/...passespython3 -m py_compileon all modified Python filespython3 -m unittest tests/test_sell_registration_metadata.py tests/test_autoresearch_worker.py tests/test_autoresearch_provenance.pypasses