Skip to content

pkg/scan still exposes no not-examined taxonomy at v2.3.4 — #391 was closed as completed but the API is unchanged #510

Closed

Description

@sectoramen

#391 asked for the internal/formatters not-examined taxonomy to be reachable by
library consumers. It was closed as completed on 2026-08-25, but adopting v2.3.4
shows the public API is unchanged. Possibly the close covered a different reading of the
ask (or a CLI-side fix), so this is a re-open rather than a complaint — with the checks
I ran, so you can point at whichever part I've got wrong.

What I verified against v2.3.4 (from the module cache, not the issue tracker)

pkg/scan.Result is byte-identical to v2.3.2:

type Result struct {
    Findings         []Finding
    Incomplete       bool // true if coverage was cut short (timeout, cancellation)
    IncompleteReason string
}

Diffing exported symbols in pkg/scan between the two versions returns nothing:

diff <(grep -hoE "^func [A-Z][A-Za-z]*|^type [A-Z][A-Za-z]*" v2.3.2/pkg/scan/*.go | sort -u) \
     <(grep -hoE "^func [A-Z][A-Za-z]*|^type [A-Z][A-Za-z]*" v2.3.4/pkg/scan/*.go | sort -u)
# (empty)

And grep -rn NotExamined pkg/ at v2.3.4 has no non-test hits — the taxonomy is still
internal/formatters only.

Why it still matters downstream

ferret-scan-mobile renders its own PDF/CSV reports and, since the last release, its own
in-app "not fully examined" tier — a file that errored or was cut short no longer counts
as clean anywhere in the UI. All of that is driven by the single Incomplete bool,
which forces two compromises:

  1. No per-file attribution. With more than one unexamined file the reason string
    degrades to "no document text extracted from 2 of 3 files", so we cannot say which
    files, or list them the way NotExaminedSummary does.
  2. NotExaminedNoText can't be honoured. Its own comment says the file's metadata
    was scanned and may already have produced findings — "Never claim this file was
    unread." A consumer holding one bool has to treat every unexamined file as fully
    unread, so our wording overclaims in one direction or the other by construction.

The prose string isn't a substitute: it still carries the internal component name and
the absolute path that NotExaminedFile.Detail promises to strip —

no document text extracted from /var/mobile/.../payroll.docx: Text Extractor: no text
extracted from .docx: no document body part was found in the archive, ...

so rendering it verbatim leaks internal vocabulary and a full path into a user-facing
document, and parsing it apart means re-deriving a taxonomy you already have.

Ask (unchanged from #391)

Export NotExaminedCause, a NotExaminedFile equivalent, and a per-result
[]NotExaminedFile alongside the existing fields, with the same cleaned Detail the
formatters get. Additive — Incomplete/IncompleteReason can stay as they are.

cmd/unscanned_report.go is documented as holding the classification because it does
filesystem I/O that must not migrate into a package the web export handler calls, so this
likely needs the classification split from the I/O rather than moved. Flagging it again
since it probably shapes the fix.

No urgency from our side — we ship the coarse version and it is correct, just blunter
than your own formatters. Happy to close this instead if the answer is "not planned";
knowing that is worth as much as the feature, because it tells us the approximation is
permanent and we should stop designing around a future API.

Related: #391 (closed completed), #296 and #336 (both closed, CLI-side).

Activity

  1. sectoramen commented on Aug 26, 2026

    @sectoramen
    ContributorAuthor

    Your verification is correct, and the diagnosis is a release gap, not an implementation gap — so
    the answer to "is the approximation permanent" is no, and you should keep designing around the API.

    Why v2.3.4 doesn't have it

    v2.3.4   tagged 2026-08-21   b249500
    9d484b0  merged 2026-08-25   fix(coverage): let the producer state why a file was not
                                 examined, and report it per file (#432, #412, #391, #417) (#482)
    

    git merge-base --is-ancestor v2.3.4 9d484b0 → true. The fix landed four days after the tag, and
    there has been no release since — 25 commits on main are unreleased, with the entry still under
    ## [Unreleased] in the CHANGELOG. So every check you ran was sound and would have returned exactly
    the same result no matter how the issue was closed. Nothing you did was wrong; you were reading the
    newest published tag, and the newest published tag predates the work.

    What is on main today

    type Result struct {
        Findings         []Finding
        Incomplete       bool
        IncompleteReason string
    
        // NotExamined lists, per file, why it was not fully examined.
        NotExamined []NotExaminedFile
    }
    
    type NotExaminedFile struct {
        Path   string
        Cause  string // one of six documented values
        Detail string // specific, payload-free
    }

    Additive, exactly as you asked — Incomplete/IncompleteReason are untouched.

    Your three compromises, checked against main

    I built your shape — a valid OOXML package with [Content_Types].xml and _rels/.rels declaring
    word/document.xml, and no such part — and ran pkg/scan.ScanFile on it:

    Incomplete            = true
    IncompleteReason      = "cannot parse: /tmp/ne/payroll.docx: Text Extractor: no text extracted…"
    findings              = 2          (docProps creator was scanned)
    NotExamined entries   = 1
      [0] Path   = "/tmp/ne/payroll.docx"
          Cause  = "cannot parse"
          Detail = "Text Extractor: no text extracted from .docx: no document body part was
                    found in the archive, so document content was NOT scanned"
    
    1. Per-file attribution — resolved. One entry per file, with its own Path. No more
      "… 2 of 3 files" degradation.
    2. NotExaminedNoText honourable — resolved. Cause carries all six values, including
      "no body text (metadata still scanned)", documented in the struct as "metadata WAS scanned and
      may already have produced findings". Worth knowing for your wording: this particular shape now
      reports "cannot parse", not the NoText cause — pkg/scan and the web UI report every coverage loss as "no document text extracted", including losses where the text WAS read #432 changed it deliberately, because a missing
      body part means document content was never scanned, so "read and found empty" overclaimed in the
      other direction. The 2 findings above are metadata, and they are still returned.
    3. The path leak — resolved in Detail. Verified programmatically: Detail contains neither the
      absolute path nor /tmp/. Only the legacy IncompleteReason still carries it, and that field is
      kept unchanged purely for compatibility.

    Two things to set expectations on, both deliberate

    Cause is a documented string, not an exported NotExaminedCause type. That deviates from the
    literal ask. The reason is in the struct comment: the taxonomy is documented for operators in
    docs/COVERAGE_DISCLOSURE.md and rendered identically by the CLI, so a caller comparing against
    what it reads in a terminal gets the same value. Treat empty as "unknown", not as any particular
    cause. If a typed enum matters for your switch exhaustiveness, say so and it can be added over the
    top without breaking the string.

    Detail still carries the internal component name (Text Extractor:). I checked whether the
    formatters clean it and pkg/scan doesn't — they don't: the CLI prints the same bytes.

    NOT FULLY EXAMINED: 1 of 1 file — findings may be missing
      cannot parse (1)
        /tmp/ne/payroll.docx  Text Extractor: no text extracted from .docx: no document body part…
    

    So Detail genuinely is "the same cleaned Detail the formatters get" — the cleaning it does is
    stripping the path, not the vocabulary. If you want the component prefix gone that is a real and
    reasonable ask, but it is a new one affecting both surfaces, and it should be its own issue rather
    than being folded in here; the CLI's wording is asserted across the suite and documented for
    operators, so changing it is a deliberate contract change.

    What you need

    A release. Until one is cut you can pin the commit directly —
    go get github.com/awslabs/ferret-scan/v2@9d484b0 — if you want to drop the coarse path early and
    confirm the shape against your PDF/CSV renderers.

    Also noting your point about cmd/unscanned_report.go: that was right, and it shaped the fix. The
    classification was split from the filesystem I/O rather than moved, which is why pkg/scan and the
    web /scan response can both carry the taxonomy without either pulling in the walk.

    Leaving this open as the release tracker rather than closing it, since the thing you are waiting on is
    real and unshipped.

  2. sectoramen commented on Aug 26, 2026

    @sectoramen
    ContributorAuthor

    v2.4.0 is released — the taxonomy is now reachable from a published version.

    https://github.com/awslabs/ferret-scan/releases/tag/v2.4.0

    go get github.com/awslabs/ferret-scan/v2@v2.4.0
    

    Also on PyPI as ferret-scan==2.4.0 (wheel and sdist), and Homebrew is in
    #511.

    Minor, so nothing you already depend on moves: the delta over v2.3.4 is 25 commits, all fixes, plus
    exactly one added exported type — NotExaminedFile. No patch release since v2.2.0 has added exported
    API in this repo, which is why this went out as a minor rather than v2.3.5.

    What you asked for, on the tag:

    type Result struct {
        Findings         []Finding
        Incomplete       bool
        IncompleteReason string
        NotExamined      []NotExaminedFile   // new
    }
    
    type NotExaminedFile struct {   // new
        Path   string
        Cause  string
        Detail string
    }

    Incomplete and IncompleteReason are untouched, so you can adopt the per-file list at your own pace
    rather than in one step.

    Two things carried over from my earlier comment, so they are not a surprise when you wire it up:

    • Cause is a documented string, not an exported enum. The six values are listed on the struct
      and are the same strings the CLI prints. Empty means no producer stated a cause — treat that as
      unknown, not as any particular cause.
    • Detail strips the absolute path but keeps the component prefix (Text Extractor: …). Verified
      again on the tag. That is byte-identical to what the CLI renders, so it genuinely is "the same
      Detail the formatters get". If you want the prefix gone, open that as its own issue — it changes
      operator-facing CLI output too, so it is a deliberate contract change rather than a tidy-up.

    Closing as completed, since the thing you were waiting on is published and verified installable
    (pip install ferret-scan==2.4.0 reports v2.4.0 (commit: 60f3fde) and detects normally).

    Reopen without hesitation if the shape does not fit your renderers — the round trip through a real
    consumer is the one part I could not test from here, and it is the part that matters. Two things this
    deliberately does not cover, so you know where the edges are rather than discovering them:

    • a typed NotExaminedCause (the ask's literal wording) — Cause is a documented string; say the word
      and it can be added additively over the top;
    • stripping the Text Extractor: component prefix from Detail, which would change operator-facing
      CLI output too and so wants its own issue.
  3. sectoramen commented on Aug 26, 2026

    @sectoramen
    ContributorAuthor

    Confirming from the downstream side, and correcting my own report: this is genuinely
    resolved in v2.4.0 (PR #482), and my "closed as completed but the API is unchanged"
    observation was accurate only against v2.3.4 — the fix landed in the next minor, not the
    patch line I was diffing against. Apologies for the noise.

    Verified after adopting v2.4.0 in ferret-scan-mobile:

    type Result struct {
        Findings         []Finding
        Incomplete       bool
        IncompleteReason string
        NotExamined      []NotExaminedFile   // new
    }

    NotExaminedFile{Path, Cause, Detail} is exactly what was needed, and two details are
    better than what I asked for:

    • Cause as a documented string rather than an int enum, so a value we render in-app
      matches what an operator reads from the CLI verbatim — no mapping table to drift.
    • Six causes, not four — "symlink not followed" and "file too large to scan" were
      both cases we were previously folding into a generic "not examined".

    The one that matters most to us is "no body text (metadata still scanned)". Holding
    only a bool, we had to treat every unexamined file as fully unread, which meant our
    wording overclaimed by construction; now it can say what actually happened.

    Adoption notes for anyone following: v2.4.0 was drop-in for us — 174 iOS and 191 macOS
    tests green, and our docx golden held at exactly 75 findings (64 high / 10 medium / 1 low)
    with an unchanged type breakdown, despite the personname/socialmedia/SSN changes in the
    same release. Thanks for building it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions