v3.27.0 review: the outfile import hardening

- The endpoint validates algorithm16 (3des2/aes, case-insensitive) and mode
  (merge/replace or omitted): a misspelled value answers 400 instead of
  silently picking 3DES2 or turning an import into a dry run.  The choices
  come from outfile.ALGORITHM16_CHOICES, derived from the algorithm map, so
  the validation and the parser cannot drift apart.
- A present but unusable PIN/PUK value is reported with its raw text: a value
  that strips to nothing (e.g. 'abcd') used to vanish without a report line,
  contradicting the "nothing is silently dropped" contract.  A column absent
  from a short row stays silent - it is not present.
- The column-line detection drops the arbitrary 8-character column-name cap:
  a candidate header must carry ICCID plus at least one column the import
  consumes (a fixed name or a numbered KIC/KID), so a long unknown column
  name next to a real one parses while 'ICCID <value>' and
  'ICCID CUSTOMER_NAME' config lines are still rejected.  Both real samples
  are unaffected.
- PWA: a chosen default keyset survives an empty or failed preview (it used
  to reset to null, and the import silently fell back to the card's first
  keyset); the select renders the preview's keysets plus a remembered value.
- PWA: the destructive Replace mode asks for confirmation first, matching the
  other store-wiping actions.
- api.md documents both validations.  Tests: the endpoint's 400s and the
  replace import, the raw-value report, the header guards, the keyset
  preservation and the declined confirmation.  906 frontend + 904 Python;
  sw cache simple-v437.

docs/api.md

@@ -307,7 +307,9 @@ ignored.
 Without a `mode` the call is a **dry run**: the parsed presets and the report
 come back for the preview (nothing is stored).  With `mode: "merge"` /
 `"replace"` the same parse runs and the presets are imported through the
-store (`replace` wipes the store first, like `/api/presets/import`).
+store (`replace` wipes the store first, like `/api/presets/import`); any other
+`mode` value is refused with `400` — a typo must not silently turn an import
+into a preview.
 
 ```json
 {"text": "…", "name": "Batch 0002", "algorithm16": "3des2",
@@ -322,7 +324,9 @@ store (`replace` wipes the store first, like `/api/presets/import`).
 - `name` — one common name applied to every imported card; empty falls back
   to the card's ICCID.
 - `algorithm16` — the algorithm nibble for a **16-byte** key: `"3des2"`
-  (default, KIc/KID nibble `5`) or `"aes"` (nibble `2`).  The key size alone
+  (default, KIc/KID nibble `5`) or `"aes"` (nibble `2`); any other value is
+  refused with `400` (a misspelled algorithm must never silently pick one —
+  the nibble decides whether the card accepts the packet).  The key size alone
   decides the others: 8 bytes → DES (`1`), 24 bytes → 3DES3 (`9`); any other
   size skips that keyset with a report line.
 - `tars` — the TAR table every imported card gets (defaults to the three role

frontend/index.html

@@ -14731,8 +14731,14 @@ function cardsOutfileRender() {
 	const o = _cardsOutfile;
 	if (!panel || !o) return;
 	const keysets = cardsOutfileKeysets();
-	const selected = keysets.includes(o.tarKeyset) ? o.tarKeyset : (keysets[0] || null);
-	o.tarKeyset = selected;
+	// An empty or failed preview must not drop a chosen keyset: the value is
+	// only re-selected against a list that has keysets, and the option list is
+	// the preview's keysets plus a remembered value (so it stays visible).
+	if (keysets.length && !keysets.includes(o.tarKeyset)) o.tarKeyset = keysets[0];
+	const options = keysets.slice();
+	if (o.tarKeyset && !options.includes(o.tarKeyset)) options.push(o.tarKeyset);
+	options.sort((a, b) => a - b);
+	const selected = o.tarKeyset;
 	let html = '<div class="flex items-center justify-between mb-2">';
 	html += '<span class="text-sm font-semibold">' + esc(t('outfile import')) +
 		': <span class="font-mono text-xs">' + esc(o.file || '') + '</span></span>';
@@ -14747,7 +14753,7 @@ function cardsOutfileRender() {
 		'<option value="aes"' + (o.algorithm16 === 'aes' ? ' selected' : '') + '>AES</option></select></label>';
 	html += '<label class="text-xs text-gray-600 dark:text-slate-400">' + esc(t('Default keyset for the TARs')) +
 		'<select id="cards-of-keyset" onchange="cardsOutfilePreview()" class="block mt-0.5 border border-gray-300 dark:border-slate-600 text-sm rounded px-2 py-1 dark:bg-slate-800">' +
-		(keysets.length ? keysets.map(v => '<option value="' + v + '"' + (v === selected ? ' selected' : '') + '>' + esc(t('Keyset') + ' ' + v) + '</option>').join('')
+		(options.length ? options.map(v => '<option value="' + v + '"' + (v === selected ? ' selected' : '') + '>' + esc(t('Keyset') + ' ' + v) + '</option>').join('')
 			: '<option value="">\u2014</option>') +
 		'</select></label>';
 	html += '<label class="text-xs text-gray-600 dark:text-slate-400">' + esc(t('Import mode')) +
@@ -14788,6 +14794,8 @@ async function cardsOutfileImport() {
 	const o = _cardsOutfile;
 	if (!o) return;
 	cardsOutfileControls();
+	// Replace wipes the whole store - the destructive choice asks first
+	if (o.mode === 'replace' && !confirm(t('Replace mode wipes the whole preset store. Continue?'))) return;
 	const btn = document.getElementById('cards-of-import');
 	if (btn) btn.disabled = true;
 	const body = {
@@ -23283,6 +23291,7 @@ const LANG_RU = {
 	'Import mode': 'Режим импорта',
 	'Merge': 'Дополнить',
 	'Replace': 'Заменить',
+	'Replace mode wipes the whole preset store. Continue?': 'Режим «Заменить» очищает всё хранилище пресетов. Продолжить?',
 	'TAR table': 'Таблица TAR',
 	'Refresh preview': 'Обновить предпросмотр',
 	'Name': 'Имя',

frontend/sw.js

@@ -1,4 +1,4 @@
-const CACHE = 'simple-v436';
+const CACHE = 'simple-v437';
 const URLS = [
   'index.html',
   'help.html',

frontend/tests/cards_outfile.test.js

@@ -45,10 +45,11 @@ code = extractConst(html, 'CARDS_TAR_DEFAULTS') + extractConst(html, 'CARDS_TAR_
 // absent (the code guards every getElementById result).
 const els = {};
 const app = new Function('els', `
-	var sandbox = { calls: [], io: [], alerts: [], fetched: 0, reply: null, replyError: null };
+	var sandbox = { calls: [], io: [], alerts: [], fetched: 0, reply: null, replyError: null, confirmResult: true };
 	var document = { getElementById: id => els[id] || null };
 	var t = k => k;
 	var alert = m => { sandbox.alerts.push(String(m)); };
+	var confirm = m => { sandbox.confirmCalls = (sandbox.confirmCalls || []); sandbox.confirmCalls.push(String(m)); return sandbox.confirmResult; };
 	var ioStatus = (id, msg) => { sandbox.io.push([id, msg]); };
 	var pysimFetchOk = async (path, body) => {
 		sandbox.calls.push({ path: path, body: body });
@@ -252,6 +253,7 @@ test('the import posts the chosen mode and refreshes the list', async () => {
 	app.calls = [];
 	app.fetched = 0;
 	app.io = [];
+	app.confirmResult = true;
 	app.reply = { ok: true, preview: false, added: 2, skipped: 1, errors: ['dup'] };
 	setEl('cards-outfile-panel', '');
 	setEl('cards-of-mode', 'replace');
@@ -266,4 +268,46 @@ test('the import posts the chosen mode and refreshes the list', async () => {
 	assert.strictEqual(app.fetched, 1);
 	assert.deepStrictEqual(app.io, [['cards-io', 'Imported 2 cards (1 skipped).']]);
 	assert.strictEqual(app.state, null);               // the panel closed
+	// the replace mode asks first, and a declined answer imports nothing
+	assert.deepStrictEqual(app.confirmCalls, ['Replace mode wipes the whole preset store. Continue?']);
+});
+
+test('a declined replace confirmation aborts the import', async () => {
+	clearEls();
+	app.calls = [];
+	app.confirmResult = false;
+	setEl('cards-of-mode', 'replace');
+	app.state = { text: TEXT, file: 'batch.out', name: 'x', algorithm16: '3des2',
+		tarKeyset: null, mode: 'merge', preview: { presets: [], report: {} } };
+	await app.cardsOutfileImport();
+	app.confirmResult = true;
+	assert.strictEqual(app.calls.length, 0);
+	assert.ok(app.state);                              // the panel stays open
+	// merge never asks
+	app.confirmResult = false;
+	setEl('cards-of-mode', 'merge');
+	app.reply = { ok: true, preview: false, added: 1, skipped: 0, errors: [] };
+	await app.cardsOutfileImport();
+	app.confirmResult = true;
+	assert.strictEqual(app.calls.length, 1);
+});
+
+test('a chosen keyset survives an empty (failed) preview', () => {
+	clearEls();
+	setEl('cards-outfile-panel', '');
+	app.state = { text: TEXT, file: 'a.out', name: 'x', algorithm16: '3des2',
+		tarKeyset: 2, mode: 'merge',
+		preview: { presets: [{ iccid: '8970119000004600098', keysets: [{ kic: '15' }, { kic: '25' }] }], report: {} } };
+	app.cardsOutfileRender();
+	assert.strictEqual(app.state.tarKeyset, 2);
+	assert.match(els['cards-outfile-panel'].innerHTML, /<option value="2" selected>/);
+	// a failed re-preview has no keysets - the choice must not be dropped
+	app.state.preview = { report: { error: 'TAR must be three hex bytes: ZZ' } };
+	app.cardsOutfileRender();
+	assert.strictEqual(app.state.tarKeyset, 2);
+	assert.match(els['cards-outfile-panel'].innerHTML, /<option value="2" selected>/);
+	// a keyset the new preview does not define falls back to its first one
+	app.state.preview = { presets: [{ iccid: '8970119000004600098', keysets: [{ kic: '15' }] }], report: {} };
+	app.cardsOutfileRender();
+	assert.strictEqual(app.state.tarKeyset, 1);
 });

pysim_simple_server/outfile.py

@@ -41,6 +41,9 @@ from pysim_simple_server import presets
 # 3DES3.  Anything else is refused with a report line.
 _KEY_ALGO = {8: 0x01, 24: 0x09}
 _KEY_ALGO_16 = {'3des2': 0x05, 'aes': 0x02}
+# The 16-byte algorithm choices as the API accepts them (derived from the map
+# above, so the endpoint's validation and the parser cannot drift apart).
+ALGORITHM16_CHOICES = tuple(_KEY_ALGO_16)
 KEY_SIZES = (8, 16, 24)
 PSK_KEY_HEX_LEN = 32        # the store's PSK contract: 32 hex chars (16 bytes)
 MAX_KEYSET = presets.KVN_MAX
@@ -58,23 +61,42 @@ def _tokens(line):
     return [p for p in parts if p]
 
 
-# A column name: a short identifier (IMSI, ICCID, KIC1, PSK_ID, ...).  The
+# A column name: an identifier (IMSI, ICCID, KIC1, PSK_ID, ...).  The shape
 # check keeps a configuration line that names a value next to a column
-# (``ICCID 897...`` - the value token is not an identifier) from being read as
-# the column line.
-_COLUMN_NAME_RE = re.compile(r'[A-Za-z][A-Za-z0-9_]{0,7}')
+# (``ICCID 897...`` - the value is not an identifier) from being read as the
+# column line.
+_COLUMN_NAME_RE = re.compile(r'[A-Za-z][A-Za-z0-9_]*')
+
+# The columns the presets use, beyond ICCID itself.  A candidate header must
+# carry at least one of them: a line with ICCID and nothing usable cannot
+# yield a card (every keyset needs a KICn/KIDn pair), so it is not the header.
+_FIXED_COLUMNS = ('ADM', 'PIN1', 'PIN2', 'PUK1', 'PUK2', 'PSK', 'PSKID', 'PSK_ID')
+
+
+def _is_known_column(name):
+    """A column the import consumes: a fixed name or a numbered KIC/KID."""
+    if name in _FIXED_COLUMNS:
+        return True
+    m = re.fullmatch(r'(?:KIC|KID)([0-9]{1,2})', name)
+    return bool(m) and 1 <= int(m.group(1)) <= MAX_KEYSET
 
 
 def find_columns(text):
     """The column-name line of an outfile: the first line whose tokens all
-    look like column names and include ``ICCID`` (at least two tokens).
-    Returns ``(columns, line_index)`` with the column names uppercased, or
+    look like column names, include ``ICCID`` and carry at least one other
+    column the import consumes (at least two tokens in total).  Returns
+    ``(columns, line_index)`` with the column names uppercased, or
     ``(None, -1)``."""
     for i, line in enumerate(str(text or '').splitlines()):
         toks = _tokens(line)
-        if (len(toks) >= 2 and any(t.upper() == 'ICCID' for t in toks)
-                and all(_COLUMN_NAME_RE.fullmatch(t) for t in toks)):
-            return [t.upper() for t in toks], i
+        if len(toks) < 2 or not all(_COLUMN_NAME_RE.fullmatch(t) for t in toks):
+            continue
+        names = [t.upper() for t in toks]
+        if 'ICCID' not in names:
+            continue
+        if not any(n != 'ICCID' and _is_known_column(n) for n in names):
+            continue
+        return names, i
     return None, -1
 
 
@@ -240,14 +262,15 @@ def parse_outfile(text, name='', algorithm16='3des2', tars=None, tar_keyset=None
         for field, col in (('pin1', 'PIN1'), ('pin2', 'PIN2'),
                            ('puk1', 'PUK1'), ('puk2', 'PUK2')):
             i = idx[col]
-            v = re.sub(r'\D', '', str(toks[i])) if 0 <= i < len(toks) else ''
-            if not v:
-                continue
+            raw = str(toks[i]).strip() if 0 <= i < len(toks) else ''
+            if not raw:
+                continue        # the column is simply absent from this row
+            v = re.sub(r'\D', '', raw)
             if re.fullmatch(r'[0-9]{4,8}', v):
                 entry[field] = v
             else:
                 skip('line %d: %s %r is not 4-8 digits - dropped'
-                     % (row_no, col, v))
+                     % (row_no, col, raw))
         i = idx['ADM']
         if 0 <= i < len(toks) and toks[i]:
             adm = _ascii_from_hex(_hex_value(toks[i]))

pysim_simple_server/server.py

@@ -10875,11 +10875,30 @@ class PysimHandler(BaseHTTPRequestHandler):
                     self._send_json(resp, 400)
                     self._log_resp(resp)
                     return
+            # A misspelled algorithm must not silently pick one: the 16-byte
+            # key's algorithm nibble decides whether the card accepts the
+            # packet at all.
+            algorithm16 = str(body.get('algorithm16') or '3des2').strip().lower()
+            if algorithm16 not in outfile.ALGORITHM16_CHOICES:
+                resp = {'error': 'algorithm16 must be one of: %s'
+                                 % ', '.join(outfile.ALGORITHM16_CHOICES)}
+                self._send_json(resp, 400)
+                self._log_resp(resp)
+                return
+            mode = body.get('mode')
+            if mode in (None, ''):
+                mode = None
+            elif mode not in ('merge', 'replace'):
+                resp = {'error': "mode must be 'merge', 'replace' or omitted "
+                                 '(a preview)'}
+                self._send_json(resp, 400)
+                self._log_resp(resp)
+                return
             try:
                 parsed = outfile.parse_outfile(
                     text,
                     name=str(body.get('name') or ''),
-                    algorithm16=str(body.get('algorithm16') or '3des2'),
+                    algorithm16=algorithm16,
                     tars=body.get('tars'),
                     tar_keyset=tar_keyset)
             except ValueError as e:
@@ -10887,8 +10906,7 @@ class PysimHandler(BaseHTTPRequestHandler):
                 self._send_json(resp, 400)
                 self._log_resp(resp)
                 return
-            mode = body.get('mode')
-            if mode not in ('merge', 'replace'):
+            if mode is None:
                 resp = {'ok': True, 'preview': True,
                         'presets': parsed['presets'], 'report': parsed['report']}
                 self._send_json(resp)

tests/test_fetch_routes.py

@@ -955,6 +955,21 @@ class PresetStoreHttpTests(unittest.TestCase):
         status, resp = self._post('/api/presets/import-outfile', {
             'text': text, 'tar_keyset': 99})
         self.assertEqual(status, 400, resp)
+        # a misspelled algorithm or mode is refused, never silently defaulted
+        status, resp = self._post('/api/presets/import-outfile', {
+            'text': text, 'algorithm16': 'aes-256'})
+        self.assertEqual(status, 400, resp)
+        self.assertIn('algorithm16', resp['error'])
+        status, resp = self._post('/api/presets/import-outfile', {
+            'text': text, 'mode': 'Merge'})
+        self.assertEqual(status, 400, resp)
+        self.assertIn('mode', resp['error'])
+        # replace wipes the store first
+        status, resp = self._post('/api/presets/import-outfile', {
+            'text': text, 'name': 'Batch 1', 'mode': 'replace'})
+        self.assertEqual(status, 200, resp)
+        self.assertEqual(resp['added'], 1)
+        self.assertEqual(len(self.store.list()), 1)
 
     def test_the_counter_an_operation_accepted_is_persisted(self):
         p = self.store.add(self._preset(iccid='8970119000004600098'))

tests/test_outfile.py

@@ -82,6 +82,21 @@ class ColumnDetectionTests(unittest.TestCase):
         self.assertEqual(res['report']['cards'], 1)
         self.assertEqual(res['presets'][0]['iccid'], DIGITS)
 
+    def test_a_header_without_a_usable_column_is_not_the_column_line(self):
+        # a line carrying ICCID and nothing the import can consume is not the
+        # header (every card needs a KICn/KIDn pair)
+        text = ('ICCID CUSTOMER_NAME\nICCID KIC1 KID1\n' + DIGITS_F + ' ' +
+                K16A + ' ' + K16B + '\n')
+        cols, i = outfile.find_columns(text)
+        self.assertEqual(i, 1)
+        self.assertEqual(cols, ['ICCID', 'KIC1', 'KID1'])
+        # ...while a long unknown column name next to a real one is fine
+        text = ('ICCID TRANSPORT_KEY_IDENTIFIER KIC1 KID1\n' + DIGITS_F + ' ' +
+                'AB ' + K16A + ' ' + K16B + '\n')
+        cols, i = outfile.find_columns(text)
+        self.assertEqual(i, 0)
+        self.assertEqual(len(cols), 4)
+
 
 class AlfaShapeTests(unittest.TestCase):
     def test_the_rows_become_presets_with_three_keysets(self):
@@ -229,3 +244,15 @@ class OptionTests(unittest.TestCase):
         self.assertEqual(len(res['presets']), 1)
         self.assertNotIn('pin1', res['presets'][0])
         self.assertTrue(any('PIN1' in s for s in res['report']['skipped']))
+        # a present but unusable value is named with its raw text (a value that
+        # strips to nothing must not vanish silently)
+        text = ('ICCID KIC1 KID1 PUK1\n' + DIGITS_F + ' ' + K16A + ' ' + K16B + ' abcd\n')
+        res = outfile.parse_outfile(text)
+        self.assertNotIn('puk1', res['presets'][0])
+        self.assertTrue(any("PUK1 'abcd'" in s for s in res['report']['skipped']))
+        # ...while a column absent from a short row stays silent (not present)
+        text = ('ICCID KIC1 KID1 PIN1 PUK1\n' + DIGITS_F + ' ' + K16A + ' ' + K16B + ' 1234\n')
+        res = outfile.parse_outfile(text)
+        self.assertEqual(res['presets'][0]['pin1'], '1234')
+        self.assertNotIn('puk1', res['presets'][0])
+        self.assertEqual([s for s in res['report']['skipped'] if 'PUK1' in s], [])