An edit can add a flash dump to a report sent without one

CLAUDE.md

@@ -182,7 +182,9 @@ restores the image but never the schema, so keep migrations additive.
   units refresh removes the board once nothing else is on it
   (`boards.RemoveAbandoned`, #452); `openipc boards remove` does it by hand. A member may edit
   their own report from `/club` (`reports.Store.Edit`, migration 031): the
-  note, the camera proposed, files out and in; ipctool's output and a backup
+  note, the camera proposed, files out and in, and a flash dump into a report
+  without one (#456: a programmer's image, or the ipctool backup taken with
+  the report's own output, with its consent); ipctool's output and a backup
   stay as sent. Like takedown it stands the guard down for its transaction,
   and every change is logged in append-only `report_edits`. An edit to a
   report already decided adds a review row `edit`: pending again, off the

frontend/apps/site/src/components/club/EditReport.test.tsx

@@ -70,3 +70,24 @@ test('a text kind says what it is, as the send forms do: the console stopped at
   expect(queryByText(/printenv/)).toBeNull();
   expect(queryByText(/from power-on to the login prompt/)).not.toBeNull();
 });
+
+test('a report without a dump can be given one, private unless its owner ticks the box', async () => {
+  const sent = capture();
+  const saved = vi.fn();
+  const { container, getByText, getByLabelText } = render(<EditReport report={report({})} t={t} onSaved={saved} onCancel={() => {}} />);
+  fireEvent.change(container.querySelector('select')!, { target: { value: 'backup' } });
+  expect(getByText(/as a programmer read it/)).toBeTruthy();
+  fireEvent.change(getByLabelText('Add', { selector: 'input' }), { target: { files: [new File([new Uint8Array(1 << 20)], 'flash.bin')] } });
+  fireEvent.click(getByText('Save'));
+  await waitFor(() => expect(saved).toHaveBeenCalled());
+  expect((sent[0].form.get('backup') as File).name).toBe('flash.bin');
+  expect(sent[0].form.get('consent')).toBe('private');
+});
+
+test('a report that has a dump is not offered another', () => {
+  capture();
+  const withDump = report({ files: [{ position: 1, kind: 'backup', name: 'backup.bin', bytes: 1 << 20, url: '', points: 10 }] });
+  const { container } = render(<EditReport report={withDump} t={t} onSaved={() => {}} onCancel={() => {}} />);
+  const values = [...container.querySelectorAll('option')].map((o) => o.value);
+  expect(values).toEqual(['photo', 'boot_log', 'uboot_env']);
+});

frontend/apps/site/src/components/club/EditReport.tsx

@@ -1,16 +1,16 @@
 /**
  * A member changing a report of theirs from /club
  * (POST /api/v1/club/reports/{id}/edit): the note, the camera they proposed,
- * files taken out, photos and text added. ipctool's output and a backup stay
- * as sent. A report a maintainer has decided goes back to them: the form says
+ * files taken out, photos and text added, and a flash dump added to a report
+ * that has none (#456). ipctool's output and a backup stay as sent. A report a maintainer has decided goes back to them: the form says
  * so before it is saved.
  */
 import { useState } from 'preact/hooks';
 import type { BoardsT } from '../../lib/boards-i18n';
 import { editReport, type MemberReport } from '../../lib/club';
 
-type AddKind = 'photo' | 'boot_log' | 'uboot_env';
-const ADD_KINDS: AddKind[] = ['photo', 'boot_log', 'uboot_env'];
+type AddKind = 'photo' | 'boot_log' | 'uboot_env' | 'backup';
+const ADD_KINDS: AddKind[] = ['photo', 'boot_log', 'uboot_env', 'backup'];
 const field = 'rounded-md border border-hairline px-2.5 py-1.5 text-sm text-body';
 
 export default function EditReport({ report, t, onSaved, onCancel }: {
@@ -23,9 +23,12 @@ export default function EditReport({ report, t, onSaved, onCancel }: {
   const [remove, setRemove] = useState<Set<number>>(new Set());
   const [kind, setKind] = useState<AddKind>('photo');
   const [add, setAdd] = useState<File[]>([]);
+  const [publicDump, setPublicDump] = useState(false);
   const [busy, setBusy] = useState(false);
   const [error, setError] = useState<string | null>(null);
   const decided = report.status === 'published' || report.status === 'rejected';
+  // One dump a report: the one it has stays as sent, so none is offered.
+  const kinds = report.files.some((f) => f.kind === 'backup') ? ADD_KINDS.filter((k) => k !== 'backup') : ADD_KINDS;
 
   const toggle = (pos: number) => {
     const next = new Set(remove);
@@ -43,6 +46,7 @@ export default function EditReport({ report, t, onSaved, onCancel }: {
     }
     for (const pos of remove) form.append('remove', String(pos));
     for (const f of add) form.append(kind, f, f.name);
+    if (kind === 'backup' && add.length > 0) form.set('consent', publicDump ? 'public' : 'private');
     setBusy(true);
     setError(null);
     editReport(report.id, form)
@@ -86,13 +90,23 @@ export default function EditReport({ report, t, onSaved, onCancel }: {
         <span class="text-body-secondary">{t('club.edit_add')}</span>
         <div class="flex flex-wrap items-center gap-2">
           <select value={kind} onChange={(e) => { setKind((e.target as HTMLSelectElement).value as AddKind); setAdd([]); }} class={field}>
-            {ADD_KINDS.map((k) => <option key={k} value={k}>{t(`club.kind_${k}`)}</option>)}
+            {kinds.map((k) => <option key={k} value={k}>{t(`club.kind_${k}`)}</option>)}
           </select>
           {/* keyed on the kind: switching it clears what was picked, input and all */}
-          <input key={kind} type="file" multiple aria-label={t('club.edit_add')} accept={kind === 'photo' ? 'image/jpeg,image/png,image/webp' : '.txt,.log,text/plain'}
+          <input key={kind} type="file" multiple={kind !== 'backup'} aria-label={t('club.edit_add')}
+            accept={kind === 'photo' ? 'image/jpeg,image/png,image/webp' : kind === 'backup' ? '' : '.txt,.log,text/plain'}
             onChange={(e) => setAdd([...((e.target as HTMLInputElement).files ?? [])])} class="text-sm text-body" />
         </div>
-        {kind !== 'photo' && <p class="m-0 text-[12px] text-body-secondary">{t(`club.send_hint_${kind}`)}</p>}
+        {(kind === 'boot_log' || kind === 'uboot_env') && <p class="m-0 text-[12px] text-body-secondary">{t(`club.send_hint_${kind}`)}</p>}
+        {kind === 'backup' && (
+          <>
+            <p class="m-0 text-[12px] text-body-secondary">{t('club.send_dump')}</p>
+            <label class="flex items-start gap-2 text-[12.5px] text-body-secondary">
+              <input type="checkbox" checked={publicDump} onChange={(e) => setPublicDump((e.target as HTMLInputElement).checked)} class="mt-0.5" />
+              {t('club.send_public')}
+            </label>
+          </>
+        )}
       </div>
       <div class="flex flex-wrap items-center gap-2">
         <button type="submit" class="site-btn site-btn-primary site-btn-sm" disabled={busy}>{t(decided ? 'club.edit_save_rereview' : 'club.edit_save')}</button>

service/internal/club/edit_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"mime/multipart"
+	"os"
 	"strconv"
 	"strings"
 	"testing"
@@ -191,3 +192,78 @@ func TestAnEditNeverSlipsPastAReviewer(t *testing.T) {
 		t.Errorf("taking out a backup: %d %v", code, d)
 	}
 }
+
+// A flash dump can be added to a report sent without one (#456): a whole
+// chip read with a programmer, or an ipctool backup taken with the report's
+// own output, asked for its consent as on sending. A decided report goes
+// back for review and the dump is paid then. One a report already has
+// stays as sent, and nothing else's dump can come in this way.
+func TestAnEditAddsAFlashDumpToAReportWithoutOne(t *testing.T) {
+	e := newEnv(t)
+	ctx := context.Background()
+	ivan := e.signedIn(t, 777, "ivan_k", "198.51.100.1")
+	maint := e.maintainer(t)
+	_, out := ivan.send(t, map[string]string{"channel": "web", "model": "anjoy-ms-j10"}, map[string][]byte{"photo": photo})
+	id := out["id"].(string)
+	if code, d := maint.json(t, "POST", "/api/v1/club/review/"+id, map[string]any{"decision": "publish"}); code != 200 || d["points"].(float64) != 1 {
+		t.Fatalf("publish: %d %v", code, d)
+	}
+	chip := bytes.Repeat([]byte{0x3c}, 1<<20)
+
+	// What is refused, leaving the report as it was.
+	if code, d := ivan.edit(t, id, map[string]string{"consent": "everyone"}, map[string][]byte{"backup": chip}); code != 400 || !strings.Contains(d["error"].(string), "consent") {
+		t.Errorf("an unknown consent: %d %v", code, d)
+	}
+	if code, d := ivan.edit(t, id, nil, map[string][]byte{"backup": bytes.Repeat([]byte{1}, 1000)}); code != 400 || !strings.Contains(d["error"].(string), "backup:") {
+		t.Errorf("neither a backup nor a chip: %d %v", code, d)
+	}
+	backup := append(append([]byte("chip:\n  vendor: HiSilicon\n"), 0), 4, 0, 0, 0, 'b', 'o', 'o', 't')
+	if code, d := ivan.edit(t, id, nil, map[string][]byte{"backup": backup}); code != 400 || !strings.Contains(d["error"].(string), "joins this one") {
+		t.Errorf("an ipctool backup on a report without ipctool's output: %d %v", code, d)
+	}
+	petr := e.signedIn(t, 888, "petr", "198.51.100.2")
+	if code, _ := petr.edit(t, id, nil, map[string][]byte{"backup": backup}); code != 404 {
+		t.Errorf("a dump on another member's report: %d", code)
+	}
+	if r := e.status(t, ivan, id); r["status"] != "published" || len(r["files"].([]any)) != 1 {
+		t.Fatalf("a refused edit changed the report: %v", r)
+	}
+
+	// The chip, public: the report goes back for review, and is paid for
+	// the dump when it is published again.
+	code, d := ivan.edit(t, id, map[string]string{"consent": "public"}, map[string][]byte{"backup": chip})
+	if code != 200 || d["status"] != "pending" || d["rereview"] != true {
+		t.Fatalf("adding the chip: %d %v", code, d)
+	}
+	var consent, name string
+	_ = e.pool.QueryRow(ctx, `SELECT r.backup_consent, f.name FROM reports r JOIN report_files f ON f.report_id = r.id AND f.kind = 'backup' WHERE r.id = $1`, id).Scan(&consent, &name)
+	if consent != "public" || name != "flash.bin" {
+		t.Errorf("the dump: consent %q, name %q", consent, name)
+	}
+	if code, d := maint.json(t, "POST", "/api/v1/club/review/"+id, map[string]any{"decision": "publish"}); code != 200 || d["points"].(float64) != 10 {
+		t.Errorf("publishing it again: %d %v", code, d)
+	}
+	// A second one is refused: the first stays as it was sent.
+	if code, d := ivan.edit(t, id, nil, map[string][]byte{"backup": bytes.Repeat([]byte{0x3d}, 1<<20)}); code != 400 || !strings.Contains(d["error"].(string), "has one already") {
+		t.Errorf("a second dump: %d %v", code, d)
+	}
+
+	// ipctool's report gets the backup ipctool took with it, and no other.
+	yaml, err := os.ReadFile("../reports/testdata/hi3516cv300-imx291.txt")
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, out = ivan.send(t, map[string]string{"channel": "web", "yaml": string(yaml)}, nil)
+	withYAML := out["id"].(string)
+	if code, d := ivan.edit(t, withYAML, nil, map[string][]byte{"backup": backup}); code != 400 || !strings.Contains(d["error"].(string), "other ipctool output") {
+		t.Errorf("a backup taken with other output: %d %v", code, d)
+	}
+	own := append(append(append([]byte{}, yaml...), 0), 4, 0, 0, 0, 'b', 'o', 'o', 't')
+	if code, d := ivan.edit(t, withYAML, nil, map[string][]byte{"backup": own}); code != 200 || d["status"] != "pending" {
+		t.Errorf("the backup taken with its output: %d %v", code, d)
+	}
+	_ = e.pool.QueryRow(ctx, `SELECT r.backup_consent, f.name FROM reports r JOIN report_files f ON f.report_id = r.id AND f.kind = 'backup' WHERE r.id = $1`, withYAML).Scan(&consent, &name)
+	if consent != "private" || name != "backup.bin" {
+		t.Errorf("the ipctool backup: consent %q, name %q", consent, name)
+	}
+}

service/internal/reports/club.go

@@ -85,10 +85,17 @@ type MemberFile struct {
 func (s *Store) Owner(ctx context.Context, id string) (string, error) {
 	var m *string
 	err := s.DB.QueryRow(ctx, `SELECT member_id FROM report_submissions WHERE report_id = $1`, id).Scan(&m)
-	if errors.Is(err, pgx.ErrNoRows) || m == nil {
+	// No submission, or one sent signed out, has no owner. A query that
+	// failed is an error, never that: m is nil then too.
+	switch {
+	case errors.Is(err, pgx.ErrNoRows):
+		return "", nil
+	case err != nil:
+		return "", err
+	case m == nil:
 		return "", nil
 	}
-	return *m, err
+	return *m, nil
 }
 
 // Mine is every report a member sent, newest first.

service/internal/reports/edit.go

@@ -15,8 +15,9 @@ import (
 )
 
 // A member changes what they sent (migration 031): the note, the camera
-// they proposed, a file taken out, photos and text added. ipctool's output
-// and a backup are what the camera said and stay as sent. An edit to a
+// they proposed, a file taken out, photos and text added, and a flash dump
+// added to a report that has none (#456). ipctool's output and a backup are
+// what the camera said and stay as sent. An edit to a
 // report a maintainer has decided puts it back in the queue -- a review row
 // 'edit', after which it is pending: off the board and the public pages
 // until it is accepted again, its stars settled by that decision.
@@ -37,6 +38,8 @@ type Change struct {
 	Proposal *Proposal
 	Remove   []int
 	Add      []File
+	// Consent is the added backup's, private or public; "" when Add has none.
+	Consent string
 }
 
 // Edited is what an edit did.
@@ -143,9 +146,35 @@ func (s *Store) Edit(ctx context.Context, id, member, by string, c Change, noteP
 			}
 			removed[pos] = true
 		}
-		if len(files)-len(removed)+len(c.Add) > maxFiles {
+		// A backup is not one of the files the limit counts, as on sending.
+		kept := 0
+		for pos, h := range files {
+			if h.kind == "backup" && c.Consent != "" {
+				return refusal{errors.New("backup: this report has one already, and it stays as it was sent")}
+			}
+			if h.kind != "backup" && !removed[pos] {
+				kept++
+			}
+		}
+		for _, f := range c.Add {
+			if f.Kind != "backup" {
+				kept++
+			}
+		}
+		if kept > maxFiles {
 			return refusal{fmt.Errorf("at most %d files per report", maxFiles)}
 		}
+		if c.Consent != "" {
+			// The consent was "none": a report without a backup had nothing
+			// to consent to. A dump added now is asked for it, as on sending.
+			tag, err := tx.Exec(ctx, `UPDATE reports SET backup_consent = $2 WHERE id = $1 AND backup_consent = 'none'`, id, c.Consent)
+			if err != nil {
+				return err
+			}
+			if tag.RowsAffected() != 1 {
+				return refusal{errors.New("backup: this report has one already, and it stays as it was sent")}
+			}
+		}
 		if hasProposal {
 			photos := 0
 			for pos, h := range files {
@@ -319,6 +348,17 @@ func (a *API) Edit(w http.ResponseWriter, r *http.Request, member, by, id string
 	}
 	c.Remove = in.removes
 
+	// Someone else's report answers as no report at all, before anything of
+	// it -- whether it holds ipctool's output, below -- can show in a
+	// refusal. Edit checks again under the report's lock.
+	if owner, err := st.Owner(ctx, id); err != nil || owner != member {
+		if err != nil {
+			a.fail(w, "the report", err)
+		} else {
+			a.refuse(w, http.StatusNotFound, "no report of yours has this id")
+		}
+		return false
+	}
 	// The report's own facts redact what the edit adds, as they did what it
 	// was sent with; any MAC is caught regardless (Redact).
 	rep, err := st.Private(ctx, id)
@@ -335,6 +375,43 @@ func (a *API) Edit(w http.ResponseWriter, r *http.Request, member, by, id string
 		_, facts, _ = Parse(rep.YAML)
 	}
 	var places []func() error
+	if in.backup != nil {
+		// A dump added later is what one sent with the report would be: an
+		// ipctool backup taken with this report's own output, or a whole
+		// chip read with a programmer. The report already says which camera
+		// it is, so the image needs no board named beside it.
+		consent := orDefault(in.fields["consent"], "private")
+		if consent != "private" && consent != "public" {
+			a.refuse(w, http.StatusBadRequest, "consent is private (OpenIPC's maintainers only) or public (published with the report)")
+			return false
+		}
+		name := "backup.bin"
+		f, err := in.backup.Open()
+		var b Backup
+		if err == nil {
+			b, err = ReadBackup(f)
+		}
+		switch {
+		case err == nil && rep.YAML == "":
+			a.refuse(w, http.StatusBadRequest, "backup: it carries ipctool's output, which an edit cannot add; send it as a new report, or from the camera with a club code that joins this one")
+			return false
+		case err == nil && Clean(rep.YAML) != Clean(b.YAML):
+			a.refuse(w, http.StatusBadRequest, "backup: it was taken with other ipctool output than this report's")
+			return false
+		case err == nil:
+		case flashImage(in.backup.Bytes):
+			name = "flash.bin"
+		default:
+			a.refuse(w, http.StatusBadRequest, "backup: "+err.Error())
+			return false
+		}
+		bf, err := a.prepare(part{kind: "backup", name: name, mime: "application/octet-stream", in: in.backup}, consent, facts, key, &places)
+		if err != nil {
+			a.fail(w, "the backup", err)
+			return false
+		}
+		c.Add, c.Consent = append(c.Add, bf), consent
+	}
 	for _, p := range in.parts {
 		f, err := a.prepare(p, "", facts, key, &places)
 		if err != nil {

service/internal/reports/handler.go

@@ -479,9 +479,9 @@ func (a *API) read(r *http.Request, edit bool) (*received, int, error) {
 				}
 				in.removes = append(in.removes, pos)
 				continue
-			case p.FileName() == "" && name != "note" && name != "maker" && name != "board" && name != "soc",
-				name == "yaml", name == "backup":
-				return in, http.StatusBadRequest, fmt.Errorf("%q cannot be edited: an edit changes the note, the camera proposed, and adds or removes photos and text files", name)
+			case p.FileName() == "" && name != "note" && name != "maker" && name != "board" && name != "soc" && name != "consent",
+				name == "yaml":
+				return in, http.StatusBadRequest, fmt.Errorf("%q cannot be edited: an edit changes the note, the camera proposed, adds or removes photos and text files, and adds a flash dump to a report without one", name)
 			}
 		}
 		// "note" is both a field (a line of text) and a file kind (a note