fix(import): forbid bad names in import DEV-2415

[rgraber]

🗒️ Checklist

1. run linter locally
2. update developer docs (API, README, inline, etc.), if any
3. for user-facing doc changes create a Zulip thread at #Support Docs Updates, if any
4. draft PR with a title <type>(<scope>)<!>: <title> DEV-1234
5. assign yourself, tag PR: at least Front end and/or Back end or workflow
6. fill in the template below and delete template comments
7. review thyself: read the diff and repro the preview as written
8. open PR & confirm that CI passes & request reviewers, if needed
9. act on any greptile review below a 5/5 score or leave comment explaining why you won't
10. delete this checklist section from the final squash commit before merging

📣 Summary

Forbid uploading assets with bad node names

📖 Description

Previously we allowed users to upload assets with spaces or other non-xml-compliant characters in the node names, then sanitized those names when we deployed, resulting in a mismatch between the asset content and what was in submissions and breaking the data table in some cases.

👀 Preview steps

Make sure to restart the kpi_worker when changing branches

1. ℹ️ have an account
2. Upload the attached xlsx to the project:
   test_upload_with_space.xlsx
3. 🔴 [on main] Upload succeeds
4. [on main] Deploy
5. [on main] Add a submission with an audio response
6. [on main] Navigate to data table and click Open on the audio file
7. 🔴 [on main] path not found / recognized
8. 🟢 [on PR] upload is forbidden

[rgraber]

@greptileai

[greptile-apps]

Greptile Summary

This PR adds a _ensure_valid_node_names validation guard to ImportTask that rejects XLSForm uploads containing survey node names with XML-invalid characters (e.g. spaces). Previously, those names were silently sanitised at deployment time, causing a mismatch between stored content and submissions that broke the data table.

• A new _ensure_valid_node_names static method iterates every survey row, skipping rows without a name or that are end-group closers, and raises ValueError for any name that fails ET.fromstring XML-tag validation. The check is wired into both the base64 upload path and the ZIP import path.
• A test exercises the base64 path with a begin group row containing spaces in its name and asserts the task transitions to ERROR status with the invalid name in the error message.

Confidence Score: 5/5

Safe to merge — the change only adds a guard that raises before any data is written, and the error propagates correctly through the existing catch-all in run().

The validation logic is straightforward: the new method correctly handles missing names (bool(name) guard), end-group rows (_is_group_end check), and delegates to the well-tested is_valid_node_name (XML parse round-trip). Both import paths are covered. The one note is a missing t() wrapper on the error string, which is a style concern that does not affect correctness or safety.

No files require special attention; the only outstanding item is the untranslated error string in import_export_task.py.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ImportTask.run] --> B[_run_task]
    B --> C{Data type?}
    C -->|base64Encoded| D[_parse_b64_upload]
    C -->|url / zip| E[_load_assets_from_url]
    E --> F[_parse_zip_import\nper asset item]
    D --> G[_b64_xls_to_dict]
    F --> H[xlsx_to_dict / xls_to_dict]
    G --> I[_ensure_valid_node_names]
    H --> I
    I --> J{survey row has\nnon-empty name?}
    J -->|No| K[Skip]
    J -->|Yes| L{_is_group_end?}
    L -->|Yes - end group row| K
    L -->|No| M{is_valid_node_name?}
    M -->|Valid XML tag| N[Continue import]
    M -->|Invalid - spaces/etc| O[raise ValueError]
    O --> P[run catches Exception\nstatus = ERROR\nmessages.error = str err]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[ImportTask.run] --> B[_run_task]
    B --> C{Data type?}
    C -->|base64Encoded| D[_parse_b64_upload]
    C -->|url / zip| E[_load_assets_from_url]
    E --> F[_parse_zip_import\nper asset item]
    D --> G[_b64_xls_to_dict]
    F --> H[xlsx_to_dict / xls_to_dict]
    G --> I[_ensure_valid_node_names]
    H --> I
    I --> J{survey row has\nnon-empty name?}
    J -->|No| K[Skip]
    J -->|Yes| L{_is_group_end?}
    L -->|Yes - end group row| K
    L -->|No| M{is_valid_node_name?}
    M -->|Valid XML tag| N[Continue import]
    M -->|Invalid - spaces/etc| O[raise ValueError]
    O --> P[run catches Exception\nstatus = ERROR\nmessages.error = str err]

Loading

_{Reviews (4): Last reviewed commit: "fixup!: changes from review" | Re-trigger Greptile}

[rgraber]

@greptileai

[platreth]

nitpick: There is a code duplicate, could pull it into a small helper!

367  survey_list = kontent.get('survey', [])
368  for node in survey_list:
369      name = node.get('name')
370      if (
371          bool(name)
372          and not _is_group_end(node)
373          and not is_valid_node_name(name)
374      ):
375          raise ValueError(f'Invalid node name: {name}')

419  survey_list = survey_dict.get('survey', [])
420  for node in survey_list:
421      name = node.get('name')
422      if bool(name) and not _is_group_end(node) and not is_valid_node_name(name):
423          raise ValueError(f'Invalid node name: {name}')