The build endpoints state their input rules once
`POST /build/publish` opened with three guard clauses and `POST /build/upload`
with eight, each one a `return render json: { error: ... }` — name present,
version present, name format, version format, file present, filename prefix,
size, digest. That is a validation layer written by hand, in a place where it
cannot be unit tested: exercising it needs a request.
`PublishInputDto` was already there and was a bare `Struct` with no rules at
all, so the controller carried them. It is an `ActiveModel::Model` now, with
the presence and platform-inclusion validations on it, and `UploadInputDto`
joins it with the name and version formats and the `<name>-<version>` filename
convention. Each controller reads:
return render json: { error: input.error_message }, status: :bad_request unless input.valid?
Size and digest keep their own explicit checks, because they are not the same
answer: 413 tells a caller to stop, 422 tells it to retry a truncated upload,
and a single error bag cannot say which. Every status code the endpoints
answered before, they answer now — build_publish_controller_spec and
build_uploads_controller_spec pin all of them, unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,7 @@ module WarpEngine
|
|||||||
api :POST, "/build/publish", "Register an uploaded build as a release"
|
api :POST, "/build/publish", "Register an uploaded build as a release"
|
||||||
header "X-Update-Secret", "Shared secret or application token (update scope)", required: true
|
header "X-Update-Secret", "Shared secret or application token (update scope)", required: true
|
||||||
param :name, String, required: true, desc: "Software name"
|
param :name, String, required: true, desc: "Software name"
|
||||||
param :platform, String, required: true, desc: "Platform (tic80, love, ebitengine, c64, godot, bevy, phaser)"
|
param :platform, String, required: true, desc: "Platform (one of WarpEngine::Platform::NAMES)"
|
||||||
param :version, String, required: true, desc: "Version string"
|
param :version, String, required: true, desc: "Version string"
|
||||||
returns code: 200, desc: "JSON with the published name/platform/version"
|
returns code: 200, desc: "JSON with the published name/platform/version"
|
||||||
error code: 401, desc: "Invalid secret"
|
error code: 401, desc: "Invalid secret"
|
||||||
@@ -21,24 +21,21 @@ module WarpEngine
|
|||||||
return render json: { error: "Unauthorized" }, status: :unauthorized
|
return render json: { error: "Unauthorized" }, status: :unauthorized
|
||||||
end
|
end
|
||||||
|
|
||||||
%i[name platform version].each do |key|
|
|
||||||
return render json: { error: "#{key.to_s.capitalize} not provided" }, status: :bad_request if params[key].blank?
|
|
||||||
end
|
|
||||||
|
|
||||||
unless software_ownership_authorized?(params[:name])
|
|
||||||
return render json: { error: "Forbidden" }, status: :forbidden
|
|
||||||
end
|
|
||||||
|
|
||||||
input = WarpEngine::PublishInputDto.new(
|
input = WarpEngine::PublishInputDto.new(
|
||||||
platform: params[:platform],
|
platform: params[:platform],
|
||||||
name: params[:name],
|
name: params[:name],
|
||||||
version: params[:version]
|
version: params[:version]
|
||||||
)
|
)
|
||||||
|
return render json: { error: input.error_message }, status: :bad_request unless input.valid?
|
||||||
|
|
||||||
|
unless software_ownership_authorized?(input.name)
|
||||||
|
return render json: { error: "Forbidden" }, status: :forbidden
|
||||||
|
end
|
||||||
|
|
||||||
WarpEngine::PublishService.new.publish(input)
|
WarpEngine::PublishService.new.publish(input)
|
||||||
claim_software_ownership(params[:name])
|
claim_software_ownership(input.name)
|
||||||
|
|
||||||
render json: { published: true, name: params[:name], platform: params[:platform], version: params[:version] }
|
render json: { published: true, name: input.name, platform: input.platform, version: input.version }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -9,8 +9,6 @@ module WarpEngine
|
|||||||
short "Build artifact upload"
|
short "Build artifact upload"
|
||||||
end
|
end
|
||||||
|
|
||||||
NAME_FORMAT = /\A[A-Za-z0-9._-]+\z/
|
|
||||||
|
|
||||||
api :POST, "/build/upload", "Upload a build artifact into the artifact directory"
|
api :POST, "/build/upload", "Upload a build artifact into the artifact directory"
|
||||||
header "X-Update-Secret", "Shared secret or application token (upload scope)", required: true
|
header "X-Update-Secret", "Shared secret or application token (upload scope)", required: true
|
||||||
param :name, String, required: true, desc: "Software name (filename must be prefixed with <name>-<version>)"
|
param :name, String, required: true, desc: "Software name (filename must be prefixed with <name>-<version>)"
|
||||||
@@ -28,35 +26,30 @@ module WarpEngine
|
|||||||
return render json: { error: "Unauthorized" }, status: :unauthorized
|
return render json: { error: "Unauthorized" }, status: :unauthorized
|
||||||
end
|
end
|
||||||
|
|
||||||
name = params[:name].to_s
|
input = WarpEngine::UploadInputDto.new(
|
||||||
version = params[:version].to_s
|
name: params[:name].to_s,
|
||||||
file = params[:file]
|
version: params[:version].to_s,
|
||||||
|
file: params[:file],
|
||||||
|
sha256: params[:sha256]
|
||||||
|
)
|
||||||
|
return render json: { error: input.error_message }, status: :bad_request unless input.valid?
|
||||||
|
|
||||||
return render json: { error: "Invalid name" }, status: :bad_request unless name.match?(NAME_FORMAT)
|
unless software_ownership_authorized?(input.name)
|
||||||
return render json: { error: "Invalid version" }, status: :bad_request unless version.match?(NAME_FORMAT)
|
|
||||||
return render json: { error: "File not provided" }, status: :bad_request unless file.respond_to?(:original_filename)
|
|
||||||
|
|
||||||
unless software_ownership_authorized?(name)
|
|
||||||
return render json: { error: "Forbidden" }, status: :forbidden
|
return render json: { error: "Forbidden" }, status: :forbidden
|
||||||
end
|
end
|
||||||
|
|
||||||
filename = File.basename(file.original_filename.to_s)
|
|
||||||
unless filename.start_with?("#{name}-#{version}.", "#{name}-#{version}-")
|
|
||||||
return render json: { error: "Filename must be prefixed with #{name}-#{version}" }, status: :bad_request
|
|
||||||
end
|
|
||||||
|
|
||||||
max = WarpEngine.config.max_upload_size
|
max = WarpEngine.config.max_upload_size
|
||||||
if file.size > max
|
if input.file.size > max
|
||||||
return render json: { error: "File too large (max #{max / (1024 * 1024)}MB)" }, status: :payload_too_large
|
return render json: { error: "File too large (max #{max / (1024 * 1024)}MB)" }, status: :payload_too_large
|
||||||
end
|
end
|
||||||
|
|
||||||
digest = Digest::SHA256.file(file.tempfile.path).hexdigest
|
digest = Digest::SHA256.file(input.file.tempfile.path).hexdigest
|
||||||
if params[:sha256].present? && !ActiveSupport::SecurityUtils.secure_compare(params[:sha256].downcase, digest)
|
if input.sha256.present? && !ActiveSupport::SecurityUtils.secure_compare(input.sha256.downcase, digest)
|
||||||
return render json: { error: "SHA256 mismatch" }, status: :unprocessable_entity
|
return render json: { error: "SHA256 mismatch" }, status: :unprocessable_entity
|
||||||
end
|
end
|
||||||
|
|
||||||
stored = WarpEngine::FileManagerService.new.upload("", file)
|
stored = WarpEngine::FileManagerService.new.upload("", input.file)
|
||||||
render json: { file: stored, size: file.size, sha256: digest }
|
render json: { file: stored, size: input.file.size, sha256: digest }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -1,7 +1,14 @@
|
|||||||
module WarpEngine
|
module WarpEngine
|
||||||
PublishInputDto = Struct.new(:platform, :name, :version, keyword_init: true) do
|
class PublishInputDto
|
||||||
def initialize(platform:, name:, version: nil)
|
include ActiveModel::Model
|
||||||
super
|
|
||||||
end
|
attr_accessor :platform, :name, :version
|
||||||
|
|
||||||
|
validates :name, presence: true
|
||||||
|
validates :version, presence: true
|
||||||
|
validates :platform, presence: true
|
||||||
|
validates :platform, inclusion: { in: WarpEngine::Platform::NAMES }, allow_blank: true
|
||||||
|
|
||||||
|
def error_message = errors.full_messages.to_sentence
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -0,0 +1,37 @@
|
|||||||
|
module WarpEngine
|
||||||
|
class UploadInputDto
|
||||||
|
include ActiveModel::Model
|
||||||
|
|
||||||
|
NAME_FORMAT = /\A[A-Za-z0-9._-]+\z/
|
||||||
|
|
||||||
|
attr_accessor :name, :version, :file, :sha256
|
||||||
|
|
||||||
|
validates :name, format: { with: NAME_FORMAT }
|
||||||
|
validates :version, format: { with: NAME_FORMAT }
|
||||||
|
validate :file_present
|
||||||
|
validate :filename_prefixed
|
||||||
|
|
||||||
|
def filename
|
||||||
|
return nil unless file.respond_to?(:original_filename)
|
||||||
|
|
||||||
|
File.basename(file.original_filename.to_s)
|
||||||
|
end
|
||||||
|
|
||||||
|
def error_message = errors.full_messages.to_sentence
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def file_present
|
||||||
|
return if file.respond_to?(:original_filename)
|
||||||
|
|
||||||
|
errors.add(:file, "not provided")
|
||||||
|
end
|
||||||
|
|
||||||
|
def filename_prefixed
|
||||||
|
return if filename.nil? || errors.include?(:name) || errors.include?(:version)
|
||||||
|
return if filename.start_with?("#{name}-#{version}.", "#{name}-#{version}-")
|
||||||
|
|
||||||
|
errors.add(:file, "name must be prefixed with #{name}-#{version}")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Reference in New Issue
Block a user