Skip to content

fix(skills): read OOXML parts as bytes and form JSON as UTF-8 in office skill scripts - #70289

Closed
solyanviktor-star wants to merge 1 commit into
NousResearch:mainfrom
solyanviktor-star:fix/office-skills-encoding
Closed

solyanviktor-star wants to merge 1 commit into
NousResearch:mainfrom
solyanviktor-star:fix/office-skills-encoding

Conversation

@solyanviktor-star

Copy link
Copy Markdown
Contributor

What does this PR do?

The office skills bundled in #68595 read user documents and agent-authored payloads with the locale-default codec, which breaks them on any non-UTF-8 host locale (stock Windows cp1251/GBK/cp932):

  1. OOXML validators (docx and powerpoint scripts/office/validators/base.py): part XML was opened in text mode and handed to lxml.etree.parse(). Live repro (cp1251): the UTF-8 bytes decode to mojibake that lxml successfully parses — validation silently runs against corrupted document text (Отчёт за июльР[icon]тчёт Р·Р° июль). On locales where the bytes don't decode, the validator crashes with UnicodeDecodeError instead of validating. Opening as bytes lets lxml honor the encoding declared in the XML prolog — the canonical way to feed lxml.

  2. PDF form scripts (fill_fillable_fields.py, fill_pdf_form_with_annotations.py, create_validation_image.py, check_bounding_boxes.py): the fields JSON is authored by the agent as UTF-8, but was read back with the locale codec — non-ASCII form values (any Cyrillic/CJK/accented input) either crash the script or get silently written into the user's PDF as mojibake. The json.dump writers use ensure_ascii=True and were already safe; only the readers needed pinning.

This is the same unwired-reader class as the merged .env/UTF-8 fixes (#60895 lineage), applied to the freshly bundled skills.

Related Issue

No open issue; found by auditing #68595 for locale-dependent I/O. Dedup: searched PRs/issues for office-skill encoding fixes — none exist.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • skills/productivity/{docx,powerpoint}/scripts/office/validators/base.py: open part XML as "rb" (lxml reads the prolog encoding).
  • skills/productivity/pdf/scripts/{fill_fillable_fields,fill_pdf_form_with_annotations,create_validation_image,check_bounding_boxes}.py: read the fields JSON with encoding="utf-8".
  • tests/skills/test_office_document_skills.py: contract test pinning every document/payload reader to a locale-independent mode, plus a live regression test that runs check_bounding_boxes.py on a non-ASCII fields.json under a forced non-UTF-8 locale (LC_ALL=C, PYTHONUTF8=0). The payload includes U+2018, whose 0x98 byte is unmapped in cp1251, so the test also fails-without-fix on Windows dev machines, not only under the POSIX C locale.

How to Test

  1. git stash the skills/ changes and run pytest tests/skills/test_office_document_skills.py -q → the 7 new tests fail (6 contract, 1 live subprocess repro).
  2. Restore the fix and re-run → 30 passed.
  3. Optional live check of the validator premise: write UTF-8 OOXML-style XML with Cyrillic text, parse via open(p, "r") + lxml.etree.parse under cp1251 → parsed text ≠ original (silent mojibake); via open(p, "rb") → equal.

…ce skill scripts

The bundled office skills (NousResearch#68595) read user documents and agent-authored
payloads with the locale-default codec:

- docx/powerpoint validators/base.py opened OOXML part XML in text mode
  before handing it to lxml. On Windows (cp1251/GBK) the bytes decode to
  mojibake that lxml then parses, so validation runs against silently
  corrupted document text; on locales where the UTF-8 bytes don't decode
  the validator crashes with UnicodeDecodeError instead of validating.
  Opening as bytes lets lxml honor the encoding declared in the XML prolog.

- The pdf form scripts (fill_fillable_fields, fill_pdf_form_with_annotations,
  create_validation_image, check_bounding_boxes) read the fields JSON the
  agent authors — UTF-8 by construction — with the locale codec, so
  non-ASCII form values (any Cyrillic/CJK/accented input) either crash or
  get written into the user's PDF as mojibake. The json.dump writers use
  ensure_ascii=True and were already safe; only the readers needed pinning.

Adds a contract test asserting every document/payload reader is
locale-independent, plus a live regression test that runs
check_bounding_boxes.py on a non-ASCII fields.json under a forced
non-UTF-8 locale — it fails without the fix on both POSIX (C locale)
and Windows (cp1251 chokes on the 0x98 byte of U+2018).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) platform/windows Native Windows-specific behavior or breakage area/i18n Localization, locales, translations sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via PR #71078 — your commit(s) were cherry-picked onto current main with your authorship preserved in git log (rebase merge). This PR was part of the class-wide close-out of bare read_text/write_text calls: all 139 remaining sites now pass explicit encoding, and a new CI linter rule prevents regressions. Thanks for the contribution!

@teknium1 teknium1 closed this Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/i18n Localization, locales, translations P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants