T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/fill_from_simplified.py:306
- Finding
- Default Workflow Silently Removes Tables and Contract Content## Vulnerability Details **File Location**: `scripts/fill_from_simplified.py`, lines 306–314, 425–426, and 434–440 **Vulnerability Type**: Destructive document processing caused by an unsafe default **Risk Level**: Medium ### Evidence ```python def remove_contract_and_tables(doc): for t in list(doc.tables): tbl = t._tbl tbl.getparent().remove(tbl) idx = find_anchor_index(doc.paragraphs, ['接待标准', '行程包含', '旅游合同', '补充协议', '合同']) if idx is not None: for p in list(doc.paragraphs[idx:]): e = p._element e.getparent().remove(e) ``` ```python if itinerary_only: remove_contract_and_tables(doc) doc.save(str(output)) ``` ```python if __name__ == '__main__': ap = argparse.ArgumentParser() ap.add_argument('--template', required=True) ap.add_argument('--content', required=True) ap.add_argument('--output', required=True) ap.add_argument('--keep-contract', action='store_true') args = ap.parse_args() fill_template(args.template, args.content, args.output, itinerary_only=(not args.keep_contract)) print(args.output) ``` ### Technical Analysis The default command-line behavior sets `itinerary_only` to true unless the caller supplies `--keep-contract`. This invokes `remove_contract_and_tables`, which performs two broad destructive operations: 1. It removes every table in the document without checking whether the table contains itinerary information. 2. It finds the first paragraph containing any broad contract-related keyword and removes that paragraph and every subsequent paragraph. This behavior conflicts with the documented requirement in `SKILL.md` to preserve template clauses and terms while replacing only itinerary-related paragraphs. A keyword match can therefore cause unrelated notices, pricing data, signatures, legal provisions, or appendices to disappear from the generated document. Th ...[truncated 1604 chars]
- Remediation
- ## Remediation Suggestions 1. Preserve all non-itinerary content by default. Make destructive removal an explicit opt-in operation. 2. Replace `--keep-contract` with a clearly named flag such as `--remove-contract-sections`, defaulting to false. 3. Do not remove every table. Identify itinerary tables through precise structural markers, bookmarks, content controls, or validated section identifiers. 4. Define both the start and end boundary of any removable section rather than deleting every paragraph after the first keyword match. 5. Require a unique anchor match and abort with a clear error if anchors are missing, duplicated, or ambiguous. 6. Emit a summary of removed elements and require confirmation before destructive processing in interactive workflows. 7. Save to a new output path and never overwrite the source template. 8. Add regression tests using templates that contain unrelated tables, contract clauses, appendices, and repeated keywords. 9. Update `SKILL.md` so the documented behavior accurately describes every modification made to the source document.