From a32e8c19f71ebc502c0d6814a2cda6cdfbda31db Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 27 Sep 2026 16:14:52 +0800 Subject: [PATCH] =?UTF-8?q?fix(api):=20=E5=A5=91=E7=BA=A6=E6=A3=80?= =?UTF-8?q?=E6=9F=A5=E5=AF=B9=E8=AF=B7=E6=B1=82=E4=BD=93=E6=8C=89=E5=AD=97?= =?UTF-8?q?=E6=AE=B5=E4=B8=A5=E6=A0=BC=E6=AF=94=E5=AF=B9=E4=B8=94=E4=B8=8D?= =?UTF-8?q?=E7=9C=8B=20Content-Type=EF=BC=8C=E8=A1=A5=E4=B8=8A=20approve?= =?UTF-8?q?=20=E7=9A=84=20expected=5Fdigest=20=E8=AF=B7=E6=B1=82=E4=BD=93?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/openapi.yaml | 18 +++++++++++++++++- internal/api/handlers_passkey_test.go | 12 ++++++------ internal/api/openapi_contract_test.go | 21 ++++++++++++++++----- panel/src/lib/openapi.gen.ts | 11 +++++++++-- 4 files changed, 48 insertions(+), 14 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 61cbcd5..e882c68 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -6571,6 +6571,20 @@ paths: security: [{ sessionCookie: [] }] parameters: - { name: id, in: path, required: true, schema: { type: string } } + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [expected_digest] + properties: + expected_digest: + type: string + description: >- + The context_sha256 the admin reviewed: the X-Felis-Context-Sha256 header of + the context they downloaded, or the listed one. A context uploaded again since + answers 409 context_changed. responses: '200': description: The approved submission, with the linked build id. @@ -6586,7 +6600,9 @@ paths: '404': $ref: '#/components/responses/NotFound' '409': - description: Submission has already been reviewed. + description: >- + The submission has already been reviewed (already_reviewed), or its context was + uploaded again after the reviewed digest (context_changed). content: application/json: schema: { $ref: '#/components/schemas/Error' } diff --git a/internal/api/handlers_passkey_test.go b/internal/api/handlers_passkey_test.go index eaf3a00..edcf075 100644 --- a/internal/api/handlers_passkey_test.go +++ b/internal/api/handlers_passkey_test.go @@ -58,7 +58,7 @@ func TestPasskeyRegisterVertical(t *testing.T) { // 1) begin returns the creation options FLAT (envelope stripped for the panel) and // stashes exactly one challenge bound to the caller. - w := do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil) + w := do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil) if w.Code != http.StatusOK { t.Fatalf("begin: code = %d, want 200 (%s)", w.Code, w.Body.String()) } @@ -157,7 +157,7 @@ func TestPasskeyBeginPassesExistingCredentials(t *testing.T) { v := &fakePasskeyVerifier{} eh := newPasskeyAPI(repo, v, user) - if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil); w.Code != http.StatusOK { + if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil); w.Code != http.StatusOK { t.Fatalf("begin: code = %d, want 200 (%s)", w.Code, w.Body.String()) } if len(v.lastUser.Credentials) != 1 || v.lastUser.Credentials[0].CredentialID != "existing-cred" { @@ -173,8 +173,8 @@ func TestPasskeyBeginSupersedes(t *testing.T) { repo := newFakeRepo() eh := newPasskeyAPI(repo, &fakePasskeyVerifier{}, user) - do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil) - do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil) + do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil) + do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil) if len(repo.passkeyChallenges) != 1 { t.Fatalf("a second begin must supersede the first; stashed challenges = %d, want 1", len(repo.passkeyChallenges)) } @@ -189,7 +189,7 @@ func TestPasskeyUnavailable(t *testing.T) { repo := newFakeRepo() eh := newPasskeyAPI(repo, nil, user) // nil verifier - if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" { + if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" { t.Fatalf("begin with no verifier: code = %d body %s, want 503 passkey_unavailable", w.Code, w.Body.String()) } if w := do(eh, "POST", "/api/v1/account/passkey/register/finish", `{"name":"x","attestation":{"a":1}}`, nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" { @@ -395,7 +395,7 @@ func TestPasskeyFaceSeparation(t *testing.T) { ih := api.InternalHandler() for _, tc := range []struct{ method, path, body string }{ - {"POST", "/api/v1/account/passkey/register/begin", `{}`}, + {"POST", "/api/v1/account/passkey/register/begin", ""}, {"POST", "/api/v1/account/passkey/register/finish", `{"name":"k","attestation":{"a":1}}`}, {"GET", "/api/v1/account/passkey/credentials", ""}, {"DELETE", "/api/v1/account/passkey/credentials/x", ""}, diff --git a/internal/api/openapi_contract_test.go b/internal/api/openapi_contract_test.go index 0a2f479..7e31b31 100644 --- a/internal/api/openapi_contract_test.go +++ b/internal/api/openapi_contract_test.go @@ -29,7 +29,8 @@ import ( // - the operation must list the status that came back (or a default / 4XX / 5XX); // - a JSON body must fit the schema documented for that status, and a response // object may carry only the properties its schema names; -// - the JSON request behind a 2xx must fit the documented requestBody. +// - the JSON request behind a 2xx must fit the documented requestBody, naming +// only the properties it names. // // Requests to a path the document does not have are left to the route parity test. @@ -163,7 +164,10 @@ func checkContract(path string, calls []contractCall) ([]string, error) { } } } - if c.status/100 == 2 && isJSON(c.reqCT) && len(c.reqBody) > 0 { + // decodeJSON reads the body whatever Content-Type says, so a JSON body sent + // without one (as most handler tests send it) is held to the document too. + reqJSON := isJSON(c.reqCT) || (c.reqCT == "" && json.Valid(c.reqBody)) + if c.status/100 == 2 && reqJSON && len(c.reqBody) > 0 { rb, ok := op["requestBody"].(map[string]any) if !ok && c.method == http.MethodGet { continue // a body on a GET is ignored, whatever it holds @@ -175,7 +179,7 @@ func checkContract(path string, calls []contractCall) ([]string, error) { if schema, ok := jsonSchemaOf(v.deref(rb)); ok { var body any if err := json.Unmarshal(c.reqBody, &body); err == nil { - for _, e := range v.check(schema, body, "request", false) { + for _, e := range v.check(schema, body, "request", true) { report(c, "%s", e) } } @@ -318,8 +322,9 @@ func (v *contractValidator) deref(n map[string]any) map[string]any { return n } -// check returns where value leaves schema. strict (responses) also refuses object -// properties the schema does not name, unless it allows additional ones. +// check returns where value leaves schema. strict also refuses object properties +// the schema does not name, unless it allows additional ones: a response field and +// a request field the document misnames (display_name for displayName) both show. func (v *contractValidator) check(schema map[string]any, value any, at string, strict bool) []string { schema = v.deref(schema) if ref, ok := schema["x-unresolved"]; ok { @@ -491,6 +496,11 @@ func TestContractCheckerCatchesDrift(t *testing.T) { // A response field outside its enum. {method: "POST", path: "/api/v1/account/link/verify", reqCT: jsonCT, reqBody: []byte(`{"code":"ABC"}`), status: 200, respCT: jsonCT, respBody: []byte(`{"linked":true,"mc_uuid":"u","auth_source":""}`), at: "j"}, + // A request property the requestBody does not name: the document once had + // display_name where the handler reads displayName. Sent with no Content-Type, + // which the handler decodes all the same. + {method: "POST", path: "/api/v1/servers", reqBody: []byte(`{"name":"x1","subdomain":"x1","display_name":"X"}`), + status: 201, respCT: jsonCT, respBody: []byte(`{"name":"x1","subdomain":"x1","desiredState":"Stopped"}`), at: "k"}, } got, err := checkContract("../../docs/openapi.yaml", calls) if err != nil { @@ -504,6 +514,7 @@ func TestContractCheckerCatchesDrift(t *testing.T) { `GET /api/v1/servers → 200 (b): response: property "extra" is not documented`, `POST /api/v1/account/link/verify → 200 (i): request.code: is integer, documented as string`, `POST /api/v1/account/link/verify → 200 (j): response.auth_source: is not one of [mojang thirdparty]`, + `POST /api/v1/servers → 201 (k): request: property "display_name" is not documented`, } if strings.Join(got, "\n") != strings.Join(want, "\n") { t.Fatalf("violations:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index 8302216..be050e6 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -8463,7 +8463,14 @@ export interface operations { }; cookie?: never; }; - requestBody?: never; + requestBody: { + content: { + "application/json": { + /** @description The context_sha256 the admin reviewed: the X-Felis-Context-Sha256 header of the context they downloaded, or the listed one. A context uploaded again since answers 409 context_changed. */ + expected_digest: string; + }; + }; + }; responses: { /** @description The approved submission, with the linked build id. */ 200: { @@ -8478,7 +8485,7 @@ export interface operations { 401: components["responses"]["Unauthorized"]; 403: components["responses"]["Forbidden"]; 404: components["responses"]["NotFound"]; - /** @description Submission has already been reviewed. */ + /** @description The submission has already been reviewed (already_reviewed), or its context was uploaded again after the reviewed digest (context_changed). */ 409: { headers: { [name: string]: unknown;