Skip to content

[Feature] : API ENDPOINTS PR 9: Files, Uploads and Acc Management endpoints - #1170

Open
pulk17 wants to merge 13 commits into
CCExtractor:masterfrom
pulk17:api-pr9-downloads
Open

[Feature] : API ENDPOINTS PR 9: Files, Uploads and Acc Management endpoints#1170
pulk17 wants to merge 13 commits into
CCExtractor:masterfrom
pulk17:api-pr9-downloads

Conversation

@pulk17

@pulk17pulk17 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

[FEATURE]

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows:

  • I am an active contributor to the project.

PR 9: Files, Uploads and Account Management

Not stacked. PR 8 (#1160) has merged, so this branches straight off master.

PR 8 closed the gaps in reading and administering the platform. This closes the
last ones: getting files out, getting samples in, and letting somebody
manage their own account. With this merged, a client can drive every part of the
platform a signed-in person can reach on the classic pages.

Thirty operations. One new route module, routes/uploads.py, for the upload
queue; everything else extends the module that already owns its prefix.

What this adds

Downloads — the classic site's file links, minus the file bytes

MethodEndpointAccess
GET/samples/{id}/downloadruns:read
GET/samples/{id}/media-info/downloadruns:read
GET/samples/{id}/extra-files/{extra_id}/downloadruns:read
GET/regression-tests/{id}/outputs/{output_id}/downloadruns:read
GET/regression-tests/{id}/outputs/{output_id}/variants/{variant_id}/downloadruns:read

These return a signed URL and a storage_status, not the file. Samples run to
gigabytes and the API should not be a transfer proxy for them; this is the shape
PR 7 established for run artifacts. When the only copy is on the platform's own
disk there is no URL to sign, so download_url is null and the status says so
rather than the request failing.

Baseline variants — a test can legitimately produce different bytes on a
different platform or CCExtractor build

MethodEndpointAccess
POST/regression-tests/{id}/outputs/{output_id}/variantsadmin/contributor + runs:write
DELETE/regression-tests/{id}/outputs/{output_id}/variants/{variant_id}admin/contributor + runs:write

Recording a hash here makes those runs pass without overwriting the baseline
everyone else is compared against, which is what promoting to baseline would do.

Sample editing

MethodEndpointAccess
GET/tagsruns:read
POST/tagsadmin + runs:write
PATCH/samples/{id}admin + runs:write
DELETE/samples/{id}admin + runs:write
DELETE/samples/{id}/extra-files/{extra_id}admin + runs:write

Uploads — the queue a file lands in before it is described

MethodEndpointAccess
POST/samples/uploadruns:write
GET/queued-samplesruns:read
GET/queued-samples/{id}runs:read
POST/queued-samples/{id}/finalizeruns:write
POST/queued-samples/{id}/linkruns:write
DELETE/queued-samples/{id}runs:write

Accounts — self-service, so these are the platform's own users rather than CI

MethodEndpointAccess
POST/auth/signuppublic
POST/auth/password-resetpublic
POST/auth/password-reset/completepublic
PATCH/auth/meany valid token, own account
GET/auth/me/ftp-credentialsruns:write, own account
GET/auth/me/githubany valid token, own account
DELETE/auth/me/githubany valid token, own account
GET/users/{id}admin + tokens:manage
POST/users/{id}/password-resetadmin, or the account's owner
POST/users/{id}/deactivateadmin, or the account's owner

Runs and platform

MethodEndpointAccess
POST/runs/{id}/restartadmin/contributor/tester + runs:write
GET/system/aboutsystem:read

GET /runs also gains a ?ccx_version= filter, which resolves a release to the
commit it was cut from so runs can be selected by version rather than by sha.

What deliberately stayed on the classic pages

Two things reach into the flow rather than sitting inside the API, and in both
cases a second implementation felt worse than the hop:

Creating an account./auth/signup mails a signed link that lands on the
existing completion page, which is where the row is actually written. Two places
able to mint accounts is a surface worth not having.

Trading a GitHub code for a token.GET /auth/me/github reports whether the
caller is connected and hands back the URL to start a connection; DELETE
forgets the platform's copy. The exchange itself stays on /github_callback,
which already holds the client secret and the redirect registered with GitHub.
The response never contains the stored token, and the authorize URL carries only
the client id and the scope, both public.

Worth stating plainly since it surprised me while writing this: GitHub here is
account linking, not sign-in. github_callback operates on an already
signed-in user, and the only place a User row is created is complete_signup.
Every account therefore has a password, so POST /auth/tokens can authenticate
all of them.

Password reset does not have to hop./auth/password-reset/complete already
takes the signed link's three values, so the only thing tying reset to these
pages was the URL in the email. A new optional CONSOLE_URL setting points the
link at a web console instead when one is deployed. Left empty, which is every
existing install, nothing changes.

Scopes

No new ones. The self-service routes carry no require_scope
PATCH /auth/me, both /auth/me/github routes, and the two /users/{id}
self-service actions. Closing your own account or asking for your own reset link
cannot depend on tokens:manage, because no role below admin may ever hold that
scope; gating them on it makes them unreachable for exactly the people who need
them. Ownership is checked in the body instead.

/auth/me/ftp-credentials is the exception and sits behind runs:write. It
hands back a working credential for the ingest server, and FTP is simply another
way to upload a sample, so a token narrowed to reading has no business fetching
one. Every role holds runs:write, so this narrows tokens without narrowing
people.

The remaining consequence, which I would rather flag than leave to be found: a
deliberately narrow token can still edit its own owner's profile. If you would
prefer it could not, the same runs:write treatment applied to the other
self-service mutations is a one-line change each.

Behaviour worth reviewing

  • The three public endpoints hold no token by definition, which is the whole
    reason for asking, so they are rate-limited per client IP rather than per
    token: five requests per fifteen minutes.
  • Changing an email or password needs the current password, not just a valid
    token. A bearer token proves the request came from a signed-in session, not
    that the sender knows the account's own credentials, so a leaked token cannot
    quietly become a stolen account.
  • Deactivation scrubs rather than deletes, and revokes the account's tokens.
    Name and email are replaced with a placeholder and the password randomised; the
    row stays so the samples and runs it owns keep an author. Scrambling the
    password only stops new tokens being minted, so the ones already issued are
    revoked here too, otherwise an account deactivated for abusing the platform
    would keep API access for up to thirty days. Tokens belonging to the caller are
    untouched unless it is the same account, so an admin doing this to somebody
    else keeps working.
  • Uploads run the same three checks as the classic form — the extension on
    the name, the mime type libmagic reads out of the file, and the extension that
    mime type implies. The third catches a banned format renamed to slip past the
    first.
  • Each upload stages under a name of its own rather than the caller's file
    name. Two clients sending sample.ts at the same moment would otherwise write
    into one file and hash something that is neither, and an endpoint invites that
    concurrency in a way the browser form never did. It also means no
    caller-supplied text reaches the staging path at all.
  • The file is hashed while it is written, because the hash is both the
    duplicate check and the name the sample is stored under. Reading a
    multi-gigabyte upload again to compute it would double the cost of every upload.
  • /queued-samples/{id}/link attaches the upload to an existing sample as an
    extra file.
    The classic link_id_confirm describes this but its body never
    does it, so this implements the described behaviour rather than the no-op.
  • Deleting a sample unlinks its files only once the row is committed, the same
    order finalize uses, so a failed commit cannot leave a sample row pointing at
    media that is already gone.
  • Restarting a run clears its results firstTestResultFile, TestResult
    and TestProgress — so a re-run replaces the old outcome instead of appending
    a second one beside it. The row is locked the way cancel locks it, so two
    restarts arriving together do not both go clearing the same results.
  • An output id belonging to a different test reads as absent._get_output
    filters on the test id and the output id together, so these URLs cannot be used
    to walk another test's baselines by guessing.
  • Variant hashes are restricted to letters and digits. The hash is joined to
    the baseline's extension to name a file under TestResults, so anything that
    could climb out of that directory is rejected at validation. Content hashes are
    hex, so this costs nothing real.
  • The password policy is read at validation time, not when the schema class is
    defined: the bounds come from MIN_PWD_LEN / MAX_PWD_LEN in config, and
    reading those during class definition would import the app mid-blueprint-setup.
    The classic forms read the same two keys, so the rules cannot drift apart.

One file outside the API

tests/test_auth/test_controllers.py is the only file here that is not
mod_api, the spec, or tests/api. test_github_redirect sets a GitHub token
on the user, which sends manage() through github_token_validity() — a live
POST to api.github.com. That failed this PR's build on a TLS interception
unrelated to any change in it, and CI has no GITHUB_CLIENT_ID to ask with in
any case, so the request went to /applications//token. It is now mocked the
same way test_github_token_validity already is, fifteen lines below.

Contract

openapi-ci-api.yaml grows from 40 paths / 49 operations to 63 / 79, following
the conventions PR 6 established: additionalProperties: false on request
bodies, with length and range limits.

Spec and implementation were cross-checked in both directions — every documented
operation is implemented and every implemented operation is documented, 79 for
79, with no dangling $refs.

Testing

83 new tests, 330 across the API suite, all passing. Beyond the success paths
they cover 400/401/403/404/409 for each endpoint, and specifically:

  • a token minted before an account is deactivated stops working immediately,
    rather than at its expiry
  • an admin deactivating somebody else keeps working on their own token
  • a read-only token is refused the FTP credentials
  • two uploads of the same file name stage to different paths, and neither path is
    named after what the caller sent
  • a file name of ... is refused rather than resolving to the staging directory
  • deleting a sample unlinks nothing until the row has actually gone
  • a variant download addressed through the wrong parent test 404s
  • an upload whose mime type is forbidden is refused even when the extension is not
  • PATCH /auth/me is refused without current_password when the change touches
    the email or the password, and 409s on an email already taken
  • a non-admin can reset and deactivate their own account but not anyone else's
  • the GitHub status response never contains the stored token, under any key
  • reset links point at these pages by default and at the console when configured
  • the real password-reset send path is exercised against a known address, not only
    the unknown-address path that returns before anything is sent

@pulk17pulk17 changed the title Api pr9 downloads[Feature] : API ENDPOINTS PR 8: Files, Uploads and Acc Management endpointsAug 12, 2026
@pulk17pulk17 changed the title [Feature] : API ENDPOINTS PR 8: Files, Uploads and Acc Management endpoints[Feature] : API ENDPOINTS PR 9: Files, Uploads and Acc Management endpointsAug 12, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@pulk17