Skip to main content

Static Code Analysis


Introduction

  • Poor coding practices are responsible for many kinds of bugs, including several common classes of security vulnerabilities. Unsafe use of user input, hardcoded/exposed security credentials, improper format strings, construction of SQL statements via string concatenation, and slow regular expressions, are all examples of tactical mistakes that have been exploited "in the wild" to compromise or disable systems.

  • While the list of "gotchas" is long and easily neglected, the good news is that many of these anti-patterns can be detected quickly and automatically by modern static code analysis tools.

  • DBOS recommends using static analysis as an ingredient in a comprehensive security strategy. As adding rule enforcement to a large, established codebase can be a hassle, DBOS recommends using tools from the beginning of a project, and therefore includes tool configuration in its demo applications and quickstart templates.

DBOS uses several techniques to ensure that static analysis is as productive as possible, with minimal hassle:

  • DBOS Transact builds on popular frameworks, thereby leveraging community best-practices and tools integration.
  • DBOS focuses on analysis rules that detect incorrect API usage and potential security vulnerabilities, rather than nitpicking on coding style.

eslint and @dbos-inc/eslint-plugin

eslint is a popular tool for checking JavaScript and TypeScript code. eslint is flexible, extensible, and comes with many standard and optional plugins. Many editors and development tools provide integration with eslint, allowing bugs to be detected early in the development cycle.

Many DBOS-suggested coding practices can be enforced by a combination of eslint plugins and rule configurations.

Installing and configuring the plugin

tip

If you got started with the quickstart, the plugin is already installed. If you encounter any errors, please make sure you install the right versions of eslint and typescript-eslint (npm will tell you if there's a peer dependency conflict).

To install the DBOS eslint plugin:

npm install --save-dev @dbos-inc/eslint-plugin

Configuring eslint can be quite involved, as there are several complete configuration schemes. Both of these options require you to set up a tsconfig.json file beforehand.

This config style will work with ESLint 8 and above.

Place an eslint.config.js file similar to the following in your project directory.

const { FlatCompat } = require("@eslint/eslintrc");
const dbosIncEslintPlugin = require("@dbos-inc/eslint-plugin");
const typescriptEslint = require("typescript-eslint");
const typescriptEslintParser = require("@typescript-eslint/parser");
const globals = require("globals");
const js = require("@eslint/js");

const compat = new FlatCompat({
baseDirectory: __dirname,
recommendedConfig: js.configs.recommended
});

module.exports = typescriptEslint.config({
extends: compat.extends("plugin:@dbos-inc/dbosRecommendedConfig"),
plugins: { "@dbos-inc": dbosIncEslintPlugin },

languageOptions: {
parser: typescriptEslintParser,
parserOptions: { project: "./tsconfig.json" },
globals: { ...globals.node, ...globals.es6 }
},

rules: { }
});

The example above configures the project for the recommended eslint configuration. Adjust the extends, rules, plugins, env, and other sections as desired, consulting the configurations and rules below.

Finally, to make eslint easy to run, it is suggested to place commands in package.json. For example:

"scripts": {
"build": "tsc",
"test": "...",
"lint": "eslint src",
"lint-fix": "eslint --fix src"
}

These rules are enabled by default:

These rules are disabled by default:

These plugins are enabled by default:


DBOS custom rules

One custom rule from DBOS, @dbos-inc/dbos-static-analysis, is provided in the @dbos-inc/eslint-plugin package. This rule is enabled by default.


Running a database query with a string that's vulnerable to SQL injection will result in an error. This would typically happen inside of a transaction.

  • SQL injection happens when a bad actor puts SQL code as a field into something like an online form, and if a programmer builds a raw query from SQL and this data, the bad actor's supposed data may allow them to run arbitrary SQL commands over your database.
  • To avoid injection, use parameterized queries. But if you accidentally make yourself vulnerable to injection, DBOS is here to save you!

Here's how you should make a parameterized query:

export class Greetings {
@Transaction()
static async InsertGreeting(ctxt: TransactionContext<Knex>, friend: string, note: string) {
await ctxt.client.raw('INSERT INTO greetings (name, note) VALUES (?, ?)', [friend, note]);
}
}

This example is vulnerable to SQL injection:

export class Greetings {
@Transaction()
static async VulnerableGreeting(ctxt: TransactionContext<Knex>, friend: string, note: string) {
await ctxt.client.raw(`INSERT INTO greetings (name, note) VALUES (${friend}, ${note})`); // Don't do this!
}
}
  • The plugin will raise a potential SQL injection error if your query string is either directly or indirectly built up of a nonliteral component.
  • For example, if your query is a format string parameterized by a function call, input variable, or other nonliteral components, the plugin will let you know that you're vulnerable to injection.
  • There has been an extensive effort to support as many literal component types as possible (things like numbers, regular expressions, ternaries, class expressions, function expressions, and some array and object literals; everything you could ever think of!). If you feel that your raw SQL query call should not be flagged as a potential SQL injection, feel free to file an issue here.

These function calls are currently flagged as nondeterministic (they may interfere with consistent workflow results or the debugger):

  • Math.random()
  • Date(), new Date(), Date.now()
  • setTimeout(...)

All such operations should use functions provided by DBOS Transact, or at a minimum, be encapsulated in a step.

These function calls are not necessarily nondeterministic, but are still warned about:

  • console.log(...)
  • bcrypt.hash(...)
  • bcrypt.compare(...)

Emitted warning messages will provide alternatives to each function call.


These behaviors result in warnings as well:

  1. Awaiting in a workflow on a non-WorkflowContext object (this implies I/O, which is often nondeterministic):

Not allowed:

@Workflow()
static async myWorkflow(ctxt: WorkflowContext) {
// Calling an external API in a workflow is not allowed.
const result = await fetch("https://www.google.com");
}

Allowed:

@Step()
static async myStep(ctxt: StepContext) {
// Calling an external API in a step is allowed.
const result = await fetch("https://www.google.com");
}
@Workflow()
static async checkoutWorkflow(ctxt: WorkflowContext) {
// All awaits start with a leftmost `WorkflowContext`.

try {
await ctxt.invoke(ShopUtilities).reserveInventory();
}
catch (error) {
ctxt.logger.error("Failed to update inventory");
await ctxt.setEvent(session_topic, null);
return;
}

// ...
}

@Workflow()
static async checkoutWorkflow(ctxt: WorkflowContext) {
/* Sometimes, you might await on a non-`WorkflowContext` object,
but the function you're calling is a helper function that uses the
underlying context. So if you pass in a `WorkflowContext` as a parameter,
the warning will be supressed. */
await doCheckout(ctxt);
}
  1. Global modification in a workflow (this often leads to nondeterministic behavior):
let globalResult = undefined;

@Workflow()
static async depositWorkflow(ctxt: WorkflowContext, data: TransactionHistory) {
globalResult = await ctxt.invoke(BankTransactionHistory).updateAcctTransactionFunc(data.toAccountId, data, true);
// ...
}

Any global variable defined outside the scope of the workflow which is directly modified will result in a warning.

  1. Malformed transactions:
  • Your transactions must have a TransactionContext<T> as the first parameter, where T is a supported database client.
  • Your transactions must also use the TransactionContext parameter's client field. You are allowed to pass your TransactionContext to a helper function as a substitute for this replacement.
  • Not meeting these requirements means that you are not using the database, which makes that transaction essentially useless. The plugin will give you a warning if so.