Skip to content

PIQ-5877: fail the build when the package carries a company name - #48

Merged
Sifaldin merged 4 commits into
masterfrom
PIQ-5877-brand-guard
Sep 30, 2026
Merged

Sifaldin merged 4 commits into
masterfrom
PIQ-5877-brand-guard

Conversation

@Sifaldin

Copy link
Copy Markdown
Contributor

Summary

Adds a build guard to this repo, the last of the four named in PIQ-5877 to have none. Cashier-React and Cashier-Integration already have theirs; PaymentIQ-Hosted-Fields gets one in a companion PR.

PIQ-5878 left this explicitly to PIQ-5877 ("SDK build guard: left to PIQ-5877").

Jira: https://paymentiq.atlassian.net/browse/PIQ-5877

What it scans, and why that choice

npm pack --dry-run --json, not the working tree.

files in package.json now limits the tarball to dist and index.d.ts, so CHANGES.md and .github/ no longer ship. Scanning the tree would fail on their GitHub links for no reason, and would miss nothing, since anything not packed is not published.

Forbidden: paymentiq, worldline, devcode, bambora, casumo, mrgreen. No allowances. This package has no brand profiles and ships one artifact, so unlike the cashier guards there is nothing a build legitimately carries.

Where it runs

Before npm publish, and on pull requests too. CI publishes on every push to master, so a branded name that gets merged is published in the same run, and a name that reaches npm cannot be withdrawn, only superseded.

Verification

Run against the current master, which PIQ-5878 made clean:

No company name in any of the 11 packed file(s).

Seeded regression, a bambora appended to README.md:

The published package carries a company name, in 1 file(s):
  README.md: bambora x1
Scanned 11 packed file(s) for: paymentiq, worldline, devcode, bambora, casumo, mrgreen

Exit codes: 0 clean, 1 a name found, 2 misconfigured (pack unreadable, no files listed, a packed file that cannot be read).

Notes

  • Not configurable from the environment. A guard that can be switched off where it runs is not a guard.
  • A packed file that cannot be read exits 2 rather than being skipped, since skipping it would be a hole.
  • FORBIDDEN entries are asserted to be word characters only, so joining them into an alternation cannot introduce a regex metacharacter later.

🤖 Generated with Claude Code

Scans what `npm pack` would ship rather than the working tree, since `files`
limits the tarball to dist and index.d.ts: scanning the tree would fail on
CHANGES.md's GitHub links for no reason.

Runs before `npm publish` and on pull requests. CI publishes on every push to
master, and a name that reaches npm cannot be withdrawn, only superseded.

Passes on master today, which PIQ-5878 made true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The guard scans file contents but misses forbidden names in packed filenames and paths.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds a CI guard preventing company names from appearing in the published package.

Changes:

  • Scans packed package contents for forbidden names.
  • Adds a reusable npm script.
  • Runs the guard before publishing and on pull requests.
File Description
scripts/​check-brand-package.mjs Implements package scanning and reporting.
package.json Adds the brand-check command.
.github/​workflows/​build_and_publish.yaml Runs the guard before publication.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/check-brand-package.mjs Outdated
Copilot review on this PR.

**The packed path is scanned as well as the contents.** A file published as
`dist/paymentiq.js` carries the name whatever is inside it, and it becomes the
consumer's import path. Relative, as npm lists it, so the checkout directory's
own path cannot trip it.

**The guard now has its own regression cases**, which is AC 1: 8 of them,
covering a clean package, each forbidden name, a name in the packed path, case
insensitivity, and the pair that proves the guard reads `npm pack` rather than
the tree, since `files` is what decides what ships.

Each fixture is a real package directory the guard runs in, because a fixture
that only wrote files would not exercise `files` at all. They run in CI ahead of
the guard, on pull requests as well as pushes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The guard is correctly scoped to published contents, tested, and enforced before CI publication.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Comment thread scripts/check-brand-package.mjs Outdated
CI publishes on every push to master, and package.json still said 1.3.3, which is
already on npm. `npm publish` would have failed on the duplicate version and
taken the master build with it.

The bump is warranted rather than a formality: package.json always ships, and
this branch adds two scripts to it, so the tarball does differ. `dist` and
`index.d.ts` are byte-identical to 1.3.3, which the changelog entry says.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Sifaldin

Copy link
Copy Markdown
Contributor Author

Good catch, and it would have broken master rather than just being untidy.

package.json said 1.3.3, which is already on npm, and CI runs npm publish on every push to master. That step would have failed on the duplicate version and taken the build with it.

The bump is also warranted on its own terms: package.json always ships, and this branch adds two scripts to it, so the tarball genuinely differs.

Fixed in d15c3ee: 1.3.4, with package-lock.json in step (npm ci rejects a mismatch) and a CHANGES.md entry that says plainly that dist and index.d.ts are byte-identical to 1.3.3, so nobody reading the changelog later thinks the SDK behaviour moved.

Ross on this PR: the pattern matched each name only as one unbroken word, so
`Payment IQ`, `Mr Green` and `payment-iq` all passed. His example is in this repo
already, `// The payment iq mid` in src/sdk/api/index.js, kept out of dist only
because uglify strips comments. README.md and index.d.ts ship unminified, so the
same spelling in either would have been published.

Each name is now stored as its parts and joined with `[\s_.-]*`, so any number of
spaces, underscores, dots or hyphens between them still matches. The
word-characters check moved to the parts.

Five regression cases added, 13 total: spaced, hyphenated and dotted spellings,
a spaced tenant name, and a spaced name in the packed path.

Releases 1.3.5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Sifaldin
Sifaldin merged commit 44af616 into master Sep 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants