fix(robots-check): stop a leading BOM or an undecodable rule from reading as allowed (#509)

* fix(robots-check): stop a leading BOM or an undecodable rule from reading as allowed

Two decoding edge cases left the robots gate failing open. Both were
flagged as follow-ups in #506.

A robots.txt saved with a UTF-8 byte-order mark decodes to a body that
starts with U+FEFF, so its first field read as U+FEFF + "user-agent". A
leading "User-agent: *" went unseen, every rule in that group was
dropped for want of an agent, and "Disallow: /" read as allow-all.
_groups() and is_robots_body() now skip one leading U+FEFF, as Google's
reference parser does, which covers every caller of allowed(). This
predates #506: the old code-page decode hid the line the same way.

Since #506 pinned curl's output to UTF-8 with errors='replace', a
non-conformant robots.txt saved in a legacy code page (cp1254,
ISO-8859-9) with raw non-ASCII bytes in a rule decodes them to U+FFFD.
That rule can never match and was silently skipped. The new
has_undecodable_rule() treats such a body as unreadable, so the gate
returns UNCONFIRMED (exit 1). The field is named with U+FFFD removed, so
a stray byte before "Disallow" cannot hide the line either. U+FFFD in a
comment or another field (Sitemap, Crawl-delay) stays harmless, and
valid UTF-8 non-ASCII rules decide as before.

Nine new tests in two FAIL-OPEN REGRESSION classes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(changelog): record the robots_check BOM and U+FFFD fixes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Can Mugan <217335573+canmugan@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -15,6 +15,25 @@ per-file diff commands.
 
 ### Fixed
 
+- **`robots_check` no longer reads a leading BOM or an undecodable rule as permission**
+  (`tools/robots_check.py`, `tests/test_robots_check.py`) - two decoding edge cases failed
+  open, both flagged as follow-ups in #506. A robots.txt saved with a UTF-8 byte-order mark
+  decodes to a body that starts with U+FEFF, so its first field was not `user-agent`: a
+  leading `User-agent: *` went unseen, every rule in that group was dropped for want of an
+  agent, and the gate read the file as allow-all. Reproduced through the CLI on real hosts:
+  cnnturk.com and sakarya.edu.tr serve `EF BB BF` + `User-agent: *`, and master printed
+  `ALLOWED` for their disallowed `/hesap/` and `/bin/`. `_groups()` and `is_robots_body()`
+  now skip one leading U+FEFF, as Google's reference parser does, so every caller of
+  `allowed()` is covered (a BOM-only body is now the empty file it is, allow-all, rather than
+  a soft 200). Separately, #506's `errors='replace'` turns raw non-ASCII bytes in a
+  non-conformant robots.txt saved in a legacy code page (cp1254, ISO-8859-9) into U+FFFD, so
+  a rule such as `Disallow: /şirket/` could never match and was silently skipped. A
+  User-agent, Allow or Disallow line holding U+FFFD now makes the body unreadable, and the
+  gate prints `UNCONFIRMED`. The field is named with U+FFFD removed, so a stray byte before
+  `Disallow` cannot hide the line either; U+FFFD in a comment or another field stays
+  harmless, and valid UTF-8 rules decide as before. Nine new tests; the seven that pin the
+  fail-opens fail on master.
+
 - **`verify_layout.py` finds an orphaned entry header by where the text starts**
   (`tools/verify_layout.py`, `tests/test_verify_layout.py`) - the orphan rule compared line
   left edges against the document margin, and a list marker moves a line's left edge without

tests/test_robots_check.py

@@ -17,7 +17,7 @@ from pathlib import Path
 REPO_ROOT = Path(__file__).resolve().parents[1]
 sys.path.insert(0, str(REPO_ROOT / "tools"))
 
-from robots_check import allowed, is_robots_body  # noqa: E402
+from robots_check import allowed, has_undecodable_rule, is_robots_body  # noqa: E402
 
 
 # Real body served by privatebank.barclays.com: blank lines sit between the
@@ -211,6 +211,134 @@ class TestPercentEncodedRules(unittest.TestCase):
         self.assertTrue(allowed(JOBUP, "*", "/en/jobs/x"))
 
 
+class TestByteOrderMark(unittest.TestCase):
+    """One leading UTF-8 byte-order mark is skipped before parsing.
+
+    FAIL-OPEN REGRESSION: a robots.txt saved with a BOM decodes to a body that
+    starts with U+FEFF, so its first field was U+FEFF + "user-agent", which is
+    not "user-agent". A leading "User-agent: *" went unseen, every rule in its
+    group was dropped for want of an agent, and "Disallow: /" read as
+    allow-all. Google's reference parser skips the BOM; Python's robotparser
+    does not.
+    """
+
+    BODY = "\ufeffUser-agent: *\nDisallow: /\n"
+
+    def test_bom_does_not_hide_the_first_group(self):
+        self.assertFalse(allowed(self.BODY, "*", "/x"))
+        self.assertFalse(allowed(self.BODY, "Claude-User", "/x"))
+
+    def test_gate_obeys_a_policy_saved_with_a_bom(self):
+        import robots_check
+
+        original = robots_check._fetch
+        robots_check._fetch = lambda url, ua: (self.BODY, 200)
+        try:
+            rc, msg = robots_check.gate("https://bom.example/jobs")
+        finally:
+            robots_check._fetch = original
+        self.assertEqual(rc, 1)
+        self.assertIn("DISALLOWED", msg)
+
+    def test_is_robots_body_skips_the_bom_too(self):
+        """A lone directive behind a BOM read as a soft 200 (fail-closed, but
+        wrong). A BOM-only file is an empty file: allow-all, as RFC 9309 says."""
+        self.assertTrue(is_robots_body("\ufeffSitemap: https://x.example/sitemap.xml\n"))
+        self.assertTrue(is_robots_body("\ufeff"))
+
+
+class TestUndecodableRules(unittest.TestCase):
+    """A User-agent, Allow or Disallow line holding U+FFFD is unreadable.
+
+    FAIL-OPEN REGRESSION: read as the UTF-8 that RFC 9309 requires, with bad
+    bytes replaced, a non-conformant robots.txt saved in cp1254 turns
+    "Disallow: /şirket/" into a pattern holding U+FFFD. It can never match, so
+    the rule was silently skipped and the path read as allowed. The gate now
+    leaves permission unconfirmed instead. U+FFFD in a comment or an unrelated
+    line decides nothing and must stay harmless.
+    """
+
+    @staticmethod
+    def _legacy(text):
+        """`text` saved in cp1254, then read as UTF-8 with bad bytes replaced."""
+        return text.encode("cp1254").decode("utf-8", "replace")
+
+    def _gate(self, body, url):
+        import robots_check
+
+        original = robots_check._fetch
+        robots_check._fetch = lambda url, ua: (body, 200)
+        try:
+            return robots_check.gate(url)
+        finally:
+            robots_check._fetch = original
+
+    def test_undecodable_disallow_is_unconfirmed_not_allowed(self):
+        body = self._legacy("User-agent: *\nDisallow: /şirket/\n")
+        self.assertIn("\ufffd", body)
+        rc, msg = self._gate(body, "https://legacy.example/şirket/x")
+        self.assertEqual(rc, 1, msg)
+        self.assertIn("UNCONFIRMED", msg)
+        self.assertIn("not valid UTF-8", msg)
+
+    def test_undecodable_star_agent_is_unconfirmed(self):
+        """A trailing 0xA0 is a no-break space in cp1254, which strip() removes,
+        but U+FFFD in UTF-8, which it does not: "*" became another agent and
+        the group's Disallow stopped applying."""
+        body = self._legacy("User-agent: *\xa0\nDisallow: /\n")
+        rc, msg = self._gate(body, "https://legacy.example/jobs")
+        self.assertEqual(rc, 1, msg)
+        self.assertIn("UNCONFIRMED", msg)
+
+    def test_undecodable_byte_before_the_field_is_unconfirmed(self):
+        """A 0xA0 indent garbled the field name, so the Disallow went unseen."""
+        body = self._legacy("User-agent: *\n\xa0Disallow: /jobs\n")
+        rc, msg = self._gate(body, "https://legacy.example/jobs")
+        self.assertEqual(rc, 1, msg)
+        self.assertIn("UNCONFIRMED", msg)
+
+    def test_every_rule_line_is_checked(self):
+        """User-agent, Allow and Disallow alike, and a first line behind a BOM:
+        _groups skips the BOM, so the check must too or it misses that line."""
+        for body in (
+            self._legacy("User-agent: Bot\xa0\n"),
+            self._legacy("Allow: /ş/\n"),
+            self._legacy("Disallow: /ş/\n"),
+            "\ufeff" + self._legacy("User-agent: *\xa0\nDisallow: /\n"),
+        ):
+            with self.subTest(body=body):
+                self.assertTrue(has_undecodable_rule(body))
+
+    def test_fffd_in_a_comment_or_an_unrelated_line_is_harmless(self):
+        body = self._legacy(
+            "# Şirket politikası\n"
+            "User-agent: *\n"
+            "Disallow: /api/  # özel\n"
+            "Sitemap: https://legacy.example/şirket.xml\n"
+            "Açıklama: iş ilanları\n"
+        )
+        # Every line but the User-agent holds U+FFFD, so the case is not vacuous.
+        clean = [line for line in body.splitlines() if "\ufffd" not in line]
+        self.assertEqual(clean, ["User-agent: *"])
+        self.assertFalse(has_undecodable_rule(body))
+        rc, msg = self._gate(body, "https://legacy.example/jobs")
+        self.assertEqual(rc, 0, msg)
+        rc, msg = self._gate(body, "https://legacy.example/api/x")
+        self.assertEqual(rc, 1, msg)
+        self.assertIn("DISALLOWED", msg)
+
+    def test_valid_utf8_non_ascii_rules_are_still_obeyed(self):
+        """Real UTF-8 never decodes to U+FFFD. The first rule is tr.indeed.com's."""
+        body = "User-agent: *\nDisallow: /職涯貼士/\nDisallow: /şirket/\n"
+        self.assertFalse(has_undecodable_rule(body))
+        for path in ("/職涯貼士/x", "/şirket/x"):
+            rc, msg = self._gate(body, "https://tr.indeed.example" + path)
+            self.assertEqual(rc, 1, msg)
+            self.assertIn("DISALLOWED", msg)
+        rc, msg = self._gate(body, "https://tr.indeed.example/cmp/x/reviews")
+        self.assertEqual(rc, 0, msg)
+
+
 
 class TestArgumentHardening(unittest.TestCase):
     """A URL can never be read by curl as an option.

tools/robots_check.py

@@ -13,9 +13,12 @@ Rules implemented (RFC 9309), deliberately on the cautious side:
   * a Disallow for either "*" or "Claude-User" blocks the retry
   * blank lines inside a record do not end it (Python's robotparser drops
     rules in that case, which fails open - see tests)
+  * one leading UTF-8 byte-order mark is skipped (left in, it hides a first
+    "User-agent" line and drops that whole group, which fails open)
   * 404 means no published policy, which is permission
   * any other failure to read robots.txt leaves permission unconfirmed,
-    and the retry does not happen
+    and the retry does not happen - including a User-agent, Allow or
+    Disallow line holding U+FFFD, i.e. a byte that is not valid UTF-8
 
 Usage:  python3 tools/robots_check.py <url>
 Exit 0 = the retry may proceed. Exit 1 = do not retry; go to escalation step 3.
@@ -56,6 +59,7 @@ def is_robots_body(text):
     body IS a valid allow-all under RFC 9309 and stays allowed; a non-empty body
     with no recognised directive is treated as unreadable.
     """
+    text = text.removeprefix('\ufeff')          # byte-order mark, see _groups
     if not text.strip():
         return True
     for raw in text.splitlines():
@@ -66,10 +70,36 @@ def is_robots_body(text):
             return True
     return False
 
+def has_undecodable_rule(text):
+    """Does a User-agent, Allow or Disallow line hold U+FFFD?
+
+    U+FFFD stands in for a byte that did not decode as UTF-8. A non-conformant
+    robots.txt saved in a legacy code page (cp1254, ISO-8859-9) with raw
+    non-ASCII bytes in a rule yields a pattern that can never match: the rule
+    was silently skipped and the path read as allowed. Such a body is
+    unreadable, not permissive. The field is named with U+FFFD removed, so a
+    stray byte before "Disallow" cannot hide the line. U+FFFD in a comment or
+    in any other field (Sitemap, Crawl-delay, ...) decides nothing and is
+    ignored.
+    """
+    for raw in text.removeprefix('\ufeff').splitlines():
+        line = raw.split('#', 1)[0]
+        if '\ufffd' in line and ':' in line:
+            field = line.split(':', 1)[0].replace('\ufffd', '').strip().lower()
+            if field in ('user-agent', 'allow', 'disallow'):
+                return True
+    return False
+
 def _groups(text):
-    """user-agent -> [(is_allow, pattern)], tolerating blank lines inside a record."""
+    """user-agent -> [(is_allow, pattern)], tolerating blank lines inside a record.
+
+    One leading U+FEFF is skipped, as Google's reference parser does. A file
+    saved with a UTF-8 byte-order mark otherwise hid its first field: a leading
+    "User-agent: *" went unseen, every rule in its group was dropped for want
+    of an agent, and "Disallow: /" read as allow-all.
+    """
     out, agents, expect = {}, [], True
-    for raw in text.splitlines():
+    for raw in text.removeprefix('\ufeff').splitlines():
         line = raw.split('#', 1)[0].strip()
         if not line or ':' not in line:
             continue
@@ -127,6 +157,9 @@ def gate(url):
             if not is_robots_body(text):
                 last = 'HTTP 200 but the body is not a robots.txt'
                 continue
+            if has_undecodable_rule(text):
+                last = 'HTTP 200 but a User-agent/Allow/Disallow line is not valid UTF-8'
+                continue
             body = text; break
         last = 'HTTP %d' % code
     if body is None: