Skip to content

feat(netsuite): add best practices SAST governance rules#3780

Open
joshOrigami wants to merge 6 commits into
semgrep:developfrom
joshOrigami:feature/netsuite-sast-rules
Open

feat(netsuite): add best practices SAST governance rules#3780
joshOrigami wants to merge 6 commits into
semgrep:developfrom
joshOrigami:feature/netsuite-sast-rules

Conversation

@joshOrigami
Copy link
Copy Markdown

Title: feat(javascript): Add NetSuite SAST governance and best practices ruleset

Description:

This PR introduces the first foundational SAST ruleset for Oracle NetSuite (SuiteScript) to the Semgrep registry.

NetSuite runs on a proprietary JavaScript API (SuiteScript 2.x) executed via a GraalVM backend. Because it is a multi-tenant cloud ERP, it enforces strict "Governance Limits" (usage units) on database operations. Standard JavaScript linting cannot catch these platform-specific governance traps, which often lead to fatal SSS_USAGE_LIMIT_EXCEEDED errors in production.

With the rapid adoption of AI-generated code (Copilot, Claude), this problem is escalating. AI models frequently write naive "Happy Path" JavaScript that works syntactically but violently violates NetSuite's database governance. This ruleset acts as a gatekeeper to prevent architectural anti-patterns from reaching production.

Rules Included in this PR:

  1. netsuite-no-record-load-save-in-loop:
    • The Risk: Loading or saving a NetSuite record inside a for/while/for each loop creates an $O(n^2)$ performance bottleneck and rapidly burns through script governance units.
    • The Fix: Forces developers to rethink the architecture and use bulk-fetch mechanisms like N/search or N/query outside the loop.
  2. netsuite-unhandled-aftersubmit-try-catch:
    • The Risk: NetSuite User Event afterSubmit triggers execute after the 'database' commit but before the user's UI resolves. An unhandled exception here can cause data inconsistency or block the user from saving the record entirely.
    • The Fix: Enforces a root-level try/catch block for mission-critical entry points.
  3. netsuite-no-console-log:
    • The Risk: Using standard browser console.log in backend SuiteScript is an audit and execution risk.
    • The Fix: Enforces the use of NetSuite's native telemetry module (N/log).

Testing:

  • Added comprehensive _test.js files covering ruleid hits and ok passes for all patterns.
  • Passed local semgrep --validate checks.

About the Author:
My name is Joshua Meiri, and I specialize in AI-Native NetSuite Engineering. I am the creator of the ABCD (Always Be Continuously Deliberate) methodology, which transitions NetSuite teams from legacy, UI-based scripting into modern, testable, MVC-driven DevOps lifecycles. I have over 20 years of experience in CRM and ERP systems and the creator of origami lens of NetSuite metadata exploration.

I built these rules because the ecosystem currently lacks a standardized, automated defense against AI-generated hallucinations and legacy messy code. By contributing this to the official registry, I hope to establish a true "financial primitive" of platform-wide safety for NetSuite developers globally.

More rules to come!

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 25, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown

@stephen-lemp stephen-lemp left a comment

Choose a reason for hiding this comment

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

Some review from a fellow SuiteScript developer. Thanks for the contribution here @joshOrigami!

references:
- https://www.origamiprecision.com/
message: "**NetSuite Reliability Risk:** afterSubmit function lacks a root
`try/catch` block. Unhandled errors here will block the record from
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The save will still occur, though the error will be raised to the user without a try-catch. I'm not sure that failing silently is a best practice here. NetSuite already wraps afterSubmit and like functions with a try-catch to ensure it doesn't crash the server, and they properly raise the error to the user.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Great catch, Stephen. You’re spot on regarding the post-commit nature of afterSubmit—the save is done, and NetSuite’s native handler does prevent a total server-hang.

My intent with this rule isn't to encourage 'silent failures' (swallowing errors), but to enforce deliberate handling. I see too many scripts that crash and leave external systems (integrations, custom records) in an inconsistent state because the dev relied on the generic NS error page instead of a structured catch that logs proper context or triggers a fallback.

I’ll refine the rule message to emphasize 'Structured Error Handling' rather than just 'Avoid Crashing.' Appreciate the sharp eye!

@@ -0,0 +1,85 @@
rules:
- id: netsuite-no-console-log
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The no-console-log rule needs to be scoped to server-side scripts only.
It should be acceptable in client scripts.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@stephen-lemp on this one I disagree, as releasing (client side) code into Production where you are potentially logging "stuff" to console is an Anti-Pattern. Agree this should be a warning.

- typescript
message: >
**NetSuite Governance Trap:** Database call (`load/save`) detected inside
a loop. This leads to O(n^2) performance and will cause
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This will not always cause SSS_USAGE_LIMIT_EXCEEDED, particularly if user includes a limit. For example, in a Suitelet the user might check number of records to load/save. If the Suitelet can cover it with 1000 governance units, it would process, otherwise it would offload to Map/Reduce. This is a valid pattern.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Spot on, Stephen. The 'Suitelet-to-M/R offload' is a classic high-perf pattern. I’m updating the message to acknowledge that usage-limit checks and offloading are the valid exceptions here.

My goal with the initial ruleset is to catch the 90% of cases where someone is just 'loop-loading' without a safety net. I’ll refine the severity and the wording so it reads more as a performance audit than a hard block. Thanks for keeping the edge cases in mind!

  • will add '- pattern-not-inside: ' for map/reduce usage.

- id: netsuite-no-console-log
languages:
- javascript
severity: ERROR
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd recommend considering this a WARNING/Medium severity

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

agree, will change.

… update: the _test.js file.

Existing:
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚗𝚘-𝚌𝚘𝚗𝚜𝚘𝚕𝚎-𝚕𝚘𝚐 — Auditing & Security (add origami-rationale)
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚗𝚘-𝚛𝚎𝚌𝚘𝚛𝚍-𝚕𝚘𝚊𝚍-𝚜𝚊𝚟𝚎-𝚒𝚗-𝚕𝚘𝚘𝚙 — Governance Protection  (add origami-rationale)
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚞𝚗𝚑𝚊𝚗𝚍𝚕𝚎𝚍-𝚊𝚏𝚝𝚎𝚛𝚜𝚞𝚋𝚖𝚒𝚝-𝚝𝚛𝚢-𝚌𝚊𝚝𝚌𝚑 — Reliability & Stability  (fix: pattern add origami-rationale)

New/Added:
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚙𝚛𝚎𝚏𝚎𝚛-𝚜𝚞𝚋𝚖𝚒𝚝-𝚏𝚒𝚎𝚕𝚍𝚜 — Targeted Body Updates
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚞𝚗𝚋𝚘𝚞𝚗𝚍𝚎𝚍-𝚜𝚎𝚊𝚛𝚌𝚑 — Scalability Protection
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚗𝚘-𝚚𝚞𝚎𝚛𝚢-𝚜𝚎𝚊𝚛𝚌𝚑-𝚒𝚗-𝚕𝚘𝚘𝚙 — Advanced Performance
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚚𝚞𝚎𝚛𝚢-𝚒𝚗𝚓𝚎𝚌𝚝𝚒𝚘𝚗 — SuiteQL Security
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚎𝚗𝚏𝚘𝚛𝚌𝚎-𝚜𝚛𝚒-𝚌𝚍𝚗 — Frontend Integrity
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚗𝚘-𝚐𝚎𝚗𝚎𝚛𝚒𝚌-𝚌𝚞𝚜𝚝𝚘𝚖-𝚒𝚍𝚜 — Clean Architecture
𝚗𝚎𝚝𝚜𝚞𝚒𝚝𝚎-𝚑𝚊𝚛𝚍𝚌𝚘𝚍𝚎𝚍-𝚒𝚗𝚝𝚎𝚛𝚗𝚊𝚕-𝚒𝚍 — Environment Portability
…ns in the Playground, the pattern variations for string concatenation in the Registry CLI are producing inconsistent parse results. I am opting for high-confidence rules only for this baseline release.
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.

3 participants