Skip to content

upload: send the report to openipc.org's board catalogue, with consent for a backup - #225

Merged
openipc-ai merged 3 commits into
masterfrom
report-upload
Sep 29, 2026
Merged

openipc-ai merged 3 commits into
masterfrom
report-upload

Conversation

@openipc-ai

@openipc-ai openipc-ai commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Restores what #78 took away. It now sends to a place people can see, and nothing leaves the camera without the owner's say.

What changes

ipctool upload used to PUT the whole flash, keyed by the camera's MAC, to the camware S3 bucket over plain HTTP. Nothing on openipc.org reads that bucket. It now sends a report to openipc.org's board catalogue (POST /api/v1/reports, OpenIPC/website#341):

  • By default, only the YAML ipctool prints. It is shown first, together with where it is going. openipc.org reviews each report before publishing it, and the public copy replaces the MAC, the die ID and the cloud ID with hashes.
  • --backup adds the whole flash in the backup file's format. The mapped partitions are sent as they are, with no copy in RAM or /tmp. The backup is private (maintainers only) unless --public is given. Either way ipctool says what the flash holds and needs a typed yes, or --yes when there is no terminal.
  • It prints the receipt address, and the catalogue board the report matches.
  • --host name[:port] points it at dev.openipc.org or a bench server. The resolver now takes an IPv4 literal as its own answer.

Removed:

  • the camware bucket;
  • its hard-coded download key;
  • restore <mac> / restore from the cloud.

restore needs a file now, and refuses before free_resources() (so before #224's isolation) when there is none.

The release job gains a step that pushes each master build to openipc.org, which serves it to stock firmware over plain HTTP (http://openipc.org/ipctool, for uget) and NFS. It stays off until the repository variable OPENIPC_ORG_TOOLS_PUSH is true, which gets set once the endpoint is live in production.

Tested

  • report_test, added to the PR check, checks the multipart body byte by byte: part framing, the backup's layout inside it, and the Content-Length. The same bytes were POSTed to the openipc.org service and accepted.
  • On the lab Hi3516EV300, against the service and then against dev.openipc.org over the internet:
    • the YAML-only report was stored;
    • a 16 MB backup went up in 7 s, and its boot, env, kernel and rootfs partitions are md5-identical to /dev/mtdblock0-3;
    • a backup with no terminal and no --yes sent nothing, and so did answering "no".
  • Rebased on restore/upgrade: stop running from the flash before rewriting it #224; native and arm builds pass.

…t for a backup

`ipctool upload` used to PUT the whole flash, keyed by the camera's MAC,
to an S3 bucket nobody reads any more, over plain HTTP, with no question
asked beyond running the command (#78). It now sends a report to
openipc.org's board catalogue, POST /api/v1/reports:

- By default only the YAML ipctool prints. It is shown first, with where
  it goes. openipc.org reviews each report before publishing it, and the
  public copy replaces the MAC, die ID and cloud ID with hashes.
- --backup adds the whole flash in the backup file's format. The mapped
  partitions are sent as they are, with no copy in RAM or /tmp. The backup
  stays private (maintainers only) unless --public is given. Either way
  ipctool says what the flash holds and needs a typed yes, or --yes when
  there is no terminal.
- It prints the receipt address, and the catalogue board the report
  matches if there is one.
- --host name[:port] points it at dev.openipc.org or a bench server. The
  resolver now takes an IPv4 literal as its own answer.

Removed: the camware bucket, its hard-coded download key, and
`restore <mac>` / `restore` from the cloud. restore needs a file now, and
refuses before it stops or unmounts anything when there is none.

report_test checks the multipart body byte by byte, the backup's layout
inside it and the Content-Length. Checked on the lab Hi3516EV300 against
the openipc.org service: the YAML-only report was stored. The 16 MB backup
went up in 7 s, and its boot, env, kernel and rootfs partitions are
md5-identical to /dev/mtdblock0-3. A backup with no terminal and no --yes
sent nothing, and so did answering "no" to the prompt.
…ock firmware

A camera on stock firmware has no curl and no TLS. openipc.org serves
ipctool over plain HTTP for uget, and over NFS, from builds this job pushes
once, over a GitHub OIDC token. The step is off until the repository
variable OPENIPC_ORG_TOOLS_PUSH is "true", which is set once the endpoint
is live in production.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Send consent-aware reports to the OpenIPC board catalogue

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Send hardware reports to the board catalogue instead of uploading flash backups to camware.
• Require explicit consent for optional flash backups; remove cloud restore and require a local
 file.
• Test multipart reports and gate release-build delivery to openipc.org behind a repository
 variable.
Diagram

graph TD
  A["Upload command"] --> B{"Include backup?"} -->|No| C["YAML report"] --> E["Multipart body"] --> F["HTTP POST"] --> G["Board catalogue"] --> H["Receipt and match"]
  B -->|Yes| D["Consent and flash"] --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Stage backup in a local file before upload
  • ➕ Could reuse the existing backup-file writer and simplify request assembly.
  • ➖ Requires camera storage for the full flash image and leaves an additional copy of sensitive data.

Recommendation: Keep the span-based upload: it reuses the backup layout without staging a large, sensitive file on a resource-constrained camera. The plain-HTTP transfer remains a material confidentiality tradeoff to review against the stock-firmware TLS constraint.

Files changed (15) +722 / -239

Enhancement (9) +544 / -236
backup.cShare backup blocks and remove cloud restore +15/-72

Share backup blocks and remove cloud restore

• Exposes the YAML-plus-flash block sequence for both file backups and report uploads. Removes camware credentials and download paths, and rejects restore requests without a readable local file before disruptive restore work begins.

src/backup.c

backup.hExpose mapped backup blocks +9/-0

Expose mapped backup blocks

• Declares the shared backup_blocks interface and documents the YAML and partition spans it returns.

src/backup.h

dns.cAccept IPv4 literals without DNS lookup +7/-0

Accept IPv4 literals without DNS lookup

• Returns a supplied IPv4 address directly, allowing uploads to bench servers addressed by IP.

src/dns.c

http.cReplace cloud transfer routines with streaming report POST +82/-152

Replace cloud transfer routines with streaming report POST

• Adds configurable-port HTTP POST with request length, span-by-span writes, progress output, and response extraction. Removes the former cloud GET and raw-backup PUT implementations.

src/http.c

http.hDeclare the span-based HTTP POST API +11/-6

Declare the span-based HTTP POST API

• Replaces cloud upload and download declarations with an HTTP POST interface that returns status and response body.

src/http.h

main.cRoute upload to the report command +18/-6

Route upload to the report command

• Adds a reusable YAML report builder and dispatches upload before general option parsing. Updates help text to describe catalogue reports and file-only restores.

src/main.c

report.cBuild multipart reports and parse catalogue answers +118/-0

Build multipart reports and parse catalogue answers

• Constructs multipart fields and optional backup data from spans while tracking total length. Parses receipt, error, and board-match details from JSON responses.

src/report.c

report.hDefine report-body and response interfaces +61/-0

Define report-body and response interfaces

• Declares bounded multipart span storage, response fields, and the report construction and command APIs.

src/report.h

report_cmd.cAdd consent-aware board report upload +223/-0

Add consent-aware board report upload

• Prints the YAML and destination before sending, then optionally adds a private or public flash backup after explicit confirmation or --yes. Supports notes and alternate hosts, posts the multipart report, and displays the receipt and board match.

src/report_cmd.c

Refactor (1) +4 / -0
dns.hMake DNS header type dependencies explicit +4/-0

Make DNS header type dependencies explicit

• Includes the standard headers needed for boolean, size, and fixed-width integer types.

src/dns.h

Tests (2) +115 / -1
pr-build-check.ymlRun report serialization tests in PR checks +3/-1

Run report serialization tests in PR checks

• Adds report_test to the native test build and execution steps.

.github/workflows/pr-build-check.yml

report_test.cVerify multipart bytes and catalogue responses +112/-0

Verify multipart bytes and catalogue responses

• Checks boundaries, fields, backup partition lengths and bytes, and calculated body size. Also tests YAML-only framing and success and error response parsing.

src/report_test.c

Documentation (1) +31 / -2
README.mdDocument catalogue uploads and backup consent +31/-2

Document catalogue uploads and backup consent

• Replaces cloud-backup usage with report-upload and local-restore instructions. Explains private and public backup choices, review, and confirmation.

README.md

Other (2) +28 / -0
release.ymlGate OIDC-authenticated build pushes to openipc.org +19/-0

Gate OIDC-authenticated build pushes to openipc.org

• Grants OIDC token permission and adds a SHA-256-tagged binary push for successful master builds. The step remains disabled unless OPENIPC_ORG_TOOLS_PUSH is true.

.github/workflows/release.yml

CMakeLists.txtBuild report modules and report_test +9/-0

Build report modules and report_test

• Adds report construction and command sources to ipctool and defines a standalone report_test executable.

CMakeLists.txt

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Flash volumes are read before approval ✓ Resolved
Description
report_cmd() calls backup_blocks() before the user confirms, and that call reads UBI volumes
into memory. On a camera with UBI-backed storage, answering no or running without a terminal
prevents the upload but only after those volumes have been collected.
Code

src/report_cmd.c[111]

+        nblocks = backup_blocks(yaml, yaml_len, blocks);
Evidence
Rule 2 requires approval before collection. The new call at line 111 precedes the confirmation at
lines 132-138; the backup callback reads UBI volumes, and read_ubi_volume() reads their contents
into an allocated buffer.

Informed approval for firmware collection
src/report_cmd.c[110-138]
src/backup.c[48-65]
src/mtd.c[206-238]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
UBI volumes are read into memory before the user approves collecting a backup.
## Fix Focus Areas
- src/report_cmd.c[107-138]
## Recommended Fix
Give the user the backup disclosure and request confirmation before calling `backup_blocks()`. If partition counts or sizes are needed for the disclosure, obtain metadata without reading volume contents.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Camera reports fail to reach the catalogue ✓ Resolved
Description
http_post() relies on common_connect() returning ERR_GENERAL for a connected socket, but that
helper closes connections completed through its nonblocking poll and returns ERR_CONNECT. When a
connection completes through that normal path, ipctool upload stops before sending either the YAML
report or a backup.
Code

src/http.c[R163-166]

+    int rc = common_connect(hostname, port, ns, &s);
+    /* common_connect() answers ERR_GENERAL when it is connected. */
+    if (rc != ERR_GENERAL)
+        return rc;
Evidence
connect_with_timeout() returns a positive poll result after an asynchronous connection succeeds.
common_connect() closes the socket on that result and returns ERR_CONNECT; the new http_post()
immediately returns that error rather than sending the report.

src/http.c[32-91]
src/http.c[109-143]
src/http.c[159-166]
src/report_cmd.c[199-208]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new report POST treats `common_connect()` as successful only when it returns `ERR_GENERAL`, but that helper closes connections completed by its nonblocking poll and reports failure.
## Fix Focus Areas
- src/http.c[32-91]
- src/http.c[109-143]
- src/http.c[159-166]
## Recommended Fix
Make connection success and failure unambiguous, preserve the successfully connected socket, and test both immediate and poll-completed connections.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. New report tests use unformatted calls ✓ Resolved
Description
report_test.c leaves several new CHECK calls on oversized lines instead of wrapping their
arguments in the repository's LLVM style. The backup-layout and response assertions contain these
calls, so subsequent edits to those checks start from inconsistent formatting.
Code

src/report_test.c[73]

+        CHECK(!memcmp(data, yaml, sizeof(yaml)), "backup does not start with the YAML and its NUL");
Evidence
Rule 8 requires changed source to follow the repository's formatting configuration. The
configuration selects LLVM style with four-space indentation, while the cited new assertions retain
oversized, unwrapped calls.

CLAUDE.md: Follow Repository Formatting Without Reformatting Generated Headers: CLAUDE.md: Follow Repository Formatting Without Reformatting Generated Headers: CLAUDE.md: Follow Repository Formatting Without Reformatting Generated Headers: CLAUDE.md: Follow Repository Formatting Without Reformatting Generated Headers
.clang-format[1-3]
src/report_test.c[73-79]
src/report_test.c[98-105]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several new report-test assertions do not follow the repository's LLVM-style formatting.
## Fix Focus Areas
- src/report_test.c[73-79]
- src/report_test.c[98-105]
## Recommended Fix
Apply the repository's `.clang-format` configuration to the new source and wrap the affected assertion calls accordingly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Owners can send an incomplete flash backup ✓ Resolved
Description
report_cmd() accepts any backup_blocks() result containing at least one flash block as a backup
of the whole flash. If opening one of several partitions fails, the mapping callback silently skips
it, so the upload proceeds with fewer partitions than the camera has.
Code

src/report_cmd.c[R110-114]

+    if (with_backup) {
+        nblocks = backup_blocks(yaml, yaml_len, blocks);
+        for (size_t i = 1; i < nblocks; i++)
+            flash += blocks[i].len;
+        if (nblocks < 2) {
Evidence
The mapping callback returns success without appending a block when open_mtdblock() fails, and
also skips unreadable UBI volumes. backup_blocks() returns only the collected count; the new
upload path rejects zero collected flash blocks but accepts every partial count.

src/backup.c[49-90]
src/backup.c[127-130]
src/report_cmd.c[110-125]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new backup upload treats one successfully collected partition as sufficient even when other flash partitions could not be opened.
## Fix Focus Areas
- src/backup.c[49-90]
- src/backup.c[127-130]
- src/report_cmd.c[110-125]
## Recommended Fix
Propagate partition-mapping and volume-read failures from the collection helper, and abort the backup upload rather than describing partial data as the whole flash.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. A refused upload kills ipctool silently ✓ Resolved
Description
write_all() in http_post() sends the request headers and multipart body with plain write(),
without suppressing SIGPIPE on the upload path. If openipc.org refuses a large backup early, such as
with a 429 or 413, and closes or resets the connection mid-upload, SIGPIPE terminates ipctool
before it can report a send error or print the server’s reason.
Code

src/http.c[R146-149]

+static int write_all(int s, const char *data, size_t len) {
+    while (len) {
+        ssize_t n = write(s, data, len);
+        if (n < 0 && errno == EINTR)
Evidence
Both the request headers and every body chunk pass through the plain-write() helper. The only
SIGPIPE suppression identified is in isolate_from_flash() on the restore/upgrade path, which
upload does not call; the PR test’s 429 refusal also shows that an early server refusal is an
expected response.

src/backup.c[592-595]
src/report_test.c[104-105]
src/http.c[146-157]
src/http.c[177-204]
src/backup.c[586-595]
src/main.c[209-210]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An early server close during an upload can raise SIGPIPE and terminate `ipctool` before it reports a send error or the server’s reason.
## Fix Focus Areas
- src/http.c[146-157]
- src/http.c[193-204]
## Recommended Fix
Suppress SIGPIPE when sending socket data, for example by using `send(s, data, len, MSG_NOSIGNAL)` in `write_all()` or ignoring SIGPIPE at the start of `http_post()`. Translate failed sends, including a close mid-body, into `ERR_SEND`; on `EPIPE` or `ECONNRESET`, still try `recv()` so an available server reason reaches the user.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View review recommended (2)
6. A public backup exposes the promised-hidden MAC ✓ Resolved
Description
The notice printed for every mode says the MAC, chip ID and cloud ID are "never shown", but with
--backup the request carries only the backup part, which starts with the raw YAML and holds the
whole flash. With --public, anyone can download that backup once it is reviewed, including those
identifiers unhashed.
Code

src/report_cmd.c[R100-105]

+    fprintf(stderr,
+            "\nThis sends the report above to http://%s%s, over plain HTTP "
+            "(stock camera firmware has no TLS).\n"
+            "It is reviewed before it is published, and the MAC, chip ID and "
+            "cloud ID in it are never shown.\n",
+            host, REPORTS_PATH);
Evidence
backup_blocks puts the raw YAML in blocks[0], and with a backup only report_add_backup is called, so
the YAML part the service hashes is never sent separately.

src/report_cmd.c[160-165]
src/backup.c[127-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The consent text promises that the identifiers are never shown, even when a public backup containing them is sent.
## Fix Focus Areas
- src/report_cmd.c[100-131]
- README.md[299-313]
## Recommended Fix
Print the 'never shown' sentence only when no public backup is sent. In the --public notice and the README, state that the backup contains the MAC, die ID, cloud ID and all flash contents in clear.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. An openipc.org outage breaks master releases ✓ Resolved
Description
The Push to openipc.org step runs failure-reporting curl -fsS commands before S3 publication
without continue-on-error, while the later S3 and Telegram steps do not use always(). Once the
push is enabled on an eligible master build, a network, endpoint, or token failure fails the matrix
job and skips those later steps even if the binary built successfully.
Code

.github/workflows/release.yml[R190-193]

+      - name: Push to openipc.org
+        if: steps.build.outcome == 'success' && env.HEAD_TAG == '' && github.ref == 'refs/heads/master' && vars.OPENIPC_ORG_TOOLS_PUSH == 'true'
+        run: |
+          T=$(curl -fsS -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \
Evidence
The push step uses failure-reporting curl commands and has no continue-on-error; it precedes the
normally gated S3 and Telegram steps, so its failure prevents them from running. The workflow
already treats Telegram as advisory with continue-on-error.

.github/workflows/release.yml[201-211]
.github/workflows/release.yml[190-203]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failure in the openipc.org push can stop an otherwise successful eligible build from reaching S3 publication and Telegram.
## Fix Focus Areas
- .github/workflows/release.yml[190-203]
## Recommended Fix
Make the openipc.org push independent of S3 publication: move it after the S3 and Telegram steps, or allow the workflow to continue after a push failure while retaining a visible failure for that push. Check that `T` is neither empty nor `null` before the PUT.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/report_cmd.c Outdated
Comment thread src/report_test.c Outdated
Comment thread src/http.c
Comment thread src/report_cmd.c Outdated
Comment thread src/http.c
Comment thread src/report_cmd.c
Comment thread .github/workflows/release.yml Outdated
… partial backups

From the review of #225:

- connect_with_timeout() returned poll()'s 1 when a non-blocking connect
  completed, and common_connect() read that as a failure. An upload
  therefore reached the server only when connect() finished at once:
  strace showed EINPROGRESS, POLLOUT, then ERR_CONNECT. It now returns 0 on
  success and -1 otherwise. Its F_GETFL result was also lost to operator
  precedence.
- The backup disclosure and its yes come before backup_blocks(), which
  reads UBI volumes into memory.
- backup_blocks() counts the partitions and volumes it could not read, and
  the upload refuses when any are missing. `backup <file>` warns.
- send(MSG_NOSIGNAL) instead of write(). A server that refuses mid-upload
  (429, 413) and closes no longer kills ipctool; its answer is read and
  printed.
- A public backup is the flash as it is. The notice and the README now say
  its identifiers are in clear; the report's own copy is hashed.
- release.yml: the openipc.org push runs last, cannot fail the job, and
  checks the token.
- clang-format on the new files.

On the lab Hi3516EV300 against dev.openipc.org: a YAML report, a private
backup, and a backup refused with no terminal. A backup sent after the
daily limit printed the server's 429 reason instead of dying on SIGPIPE.
@openipc-ai
openipc-ai merged commit e7fb628 into master Sep 29, 2026
5 checks passed
@openipc-ai
openipc-ai deleted the report-upload branch September 29, 2026 14:58
openipc-ai added a commit that referenced this pull request Sep 29, 2026
curl -f fails only on 400 and above. After #225 the push got nginx's 302 to
openipc.org's home page (the endpoint had no location yet) and the step
reported success. It now checks for 200, prints the answer, and fails
visibly otherwise; continue-on-error still keeps it from failing the
build.
Sign up for free to 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