Skip to content

Format with Prettier#115

Open
mwojick-keeper wants to merge 3 commits intomainfrom
prettier
Open

Format with Prettier#115
mwojick-keeper wants to merge 3 commits intomainfrom
prettier

Conversation

@mwojick-keeper
Copy link
Contributor

These are the only real changes:
2494f53
2df3f2c

The rest is formatting from npx prettier --write .

The format commit (and others added to .git-blame-ignore-revs) can be ignored in the blame by running:
git config blame.ignoreRevsFile .git-blame-ignore-revs

@mwojick-keeper mwojick-keeper changed the title Prettier Format with Prettier Mar 5, 2026
Comment on lines +7 to +55
name: Audit Project

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3

- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: '24'

- name: Lib - Install
run: npm i
working-directory: ./keeperapi
env:
NPM_TOKEN: ''

- name: Lib - Build
run: npm run build
working-directory: ./keeperapi
env:
NPM_TOKEN: ''

- name: Lib - Check Types
run: npm run types:ci
working-directory: ./keeperapi
env:
NPM_TOKEN: ''

- name: Lib - Run Unit Tests
run: npm run test
working-directory: ./keeperapi

- name: Examples (node) - Installation
run: npm run link-local
working-directory: ./examples/print-vault-node

- name: Examples (node) - Check Types
run: npm run types:ci
working-directory: ./examples/print-vault-node

- name: Examples (browser) - Installation
run: npm run link-local
working-directory: ./examples/print-vault-browser

- name: Examples (browser) - Check Types
run: npm run types:ci
working-directory: ./examples/print-vault-browser

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 4 days ago

To fix the problem, explicitly declare restricted GITHUB_TOKEN permissions in the workflow. The safest and simplest approach here is to add a permissions: block at the workflow (top) level, which will apply to the audit job and any future jobs that don’t override it. Since the workflow only needs to read repository contents, we can set contents: read, which matches the minimal permissions CodeQL suggested.

Concretely, in .github/workflows/main.yml, add a permissions: block after the on: definition and before jobs:. No existing steps or functionality need to change; the workflow will still be triggered on push, run on ubuntu-latest, and execute the same npm commands, but with a GITHUB_TOKEN limited to reading repository contents. No imports, extra methods, or additional configuration files are required.

Suggested changeset 1
.github/workflows/main.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -2,6 +2,9 @@
 
 on: [push]
 
+permissions:
+    contents: read
+
 jobs:
     audit:
         name: Audit Project
EOF
@@ -2,6 +2,9 @@

on: [push]

permissions:
contents: read

jobs:
audit:
name: Audit Project
Copilot is powered by AI and may make mistakes. Always verify output.
import LoginType = Authentication.LoginType;

process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';

Check failure

Code scanning / CodeQL

Disabling certificate validation High

Disabling certificate validation is strongly discouraged.

Copilot Autofix

AI 4 days ago

In general, the fix is to stop globally disabling TLS certificate validation via NODE_TLS_REJECT_UNAUTHORIZED. Instead, allow Node.js to perform default certificate verification or, if a custom trust setup is required (for example, self-signed certs in a dev environment), configure trusted CAs or TLS options in a narrowly scoped manner without turning verification off.

For this specific file, the minimal, behavior-preserving fix that removes the insecure practice is to delete the line process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';. The rest of the code will continue to function using Node.js’s default TLS behavior, relying on the certificates presented by KeeperEnvironment.DEV (or whatever host is used) being valid and trusted by the runtime environment. No other changes, imports, or definitions are needed, because this line is not required for program flow or logic—its only effect is weakening security. The change is localized to line 5 of examples/print-vault-node/src/index.ts.

Suggested changeset 1
examples/print-vault-node/src/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/examples/print-vault-node/src/index.ts b/examples/print-vault-node/src/index.ts
--- a/examples/print-vault-node/src/index.ts
+++ b/examples/print-vault-node/src/index.ts
@@ -2,8 +2,6 @@
 import * as readline from 'readline';
 import LoginType = Authentication.LoginType;
 
-process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
-
 const clientVersion = 'w16.4.0';
 
 async function printVault(username: string, password: string) {
EOF
@@ -2,8 +2,6 @@
import * as readline from 'readline';
import LoginType = Authentication.LoginType;

process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';

const clientVersion = 'w16.4.0';

async function printVault(username: string, password: string) {
Copilot is powered by AI and may make mistakes. Always verify output.
}
keyBytesCache[keyId] = keyBytes
return keyBytes
keyBytesCache[keyId] = keyBytes;

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
cryptoKeysCache[keyType][keyId] = key
return key
const key = await this.loadCryptoKey(keyId, keyType, storage);
cryptoKeysCache[keyType][keyId] = key;

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
const { data, dataId, keyId, encryptionType } = task;
try {
const keyBytes = await platform.decrypt(data, keyId, encryptionType, keyStorage);
results[dataId] = keyBytes;

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
keyCache[keyId] = keyBytes
return keyBytes
}
keyCache[keyId] = keyBytes;

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
cryptoKeysCache[keyType][keyId] = key
return key
const key = await this.loadCryptoKey(keyId, keyType, storage);
cryptoKeysCache[keyType][keyId] = key;

Check warning

Code scanning / CodeQL

Prototype-polluting assignment Medium

This assignment may alter Object.prototype if a malicious '__proto__' string is injected from
user controlled input
.

Copilot Autofix

AI 4 days ago

In general, the fix is to ensure that the property name used for indexing cryptoKeysCache cannot be a magic prototype key (__proto__, constructor, prototype) or any unexpected string. The two common approaches are: (1) validate and restrict keyType to a known safe set of strings at runtime before using it as an object key, or (2) replace the plain object used as a cache with a prototype-less object or a Map so that even if an attacker supplies __proto__, it cannot affect Object.prototype.

For this code, the minimal, non–behaviour-changing fix is to add a runtime guard in decrypt (where the tainted encryptionType first enters this logic) that ensures it is one of the expected literal values ('cbc' | 'gcm' | 'rsa' | 'ecc'). If it is anything else, we throw an error before calling loadKey. This guarantees that loadKey and loadCryptoKey only ever receive safe keyType values, removing the possibility that cryptoKeysCache[keyType] could become cryptoKeysCache.__proto__. To further harden cryptoKeysCache itself against accidental pollution, we can also switch its declaration to a prototype-less object (created with Object.create(null)) in the snippet we control, but since that declaration is not shown, we will solve the issue solely via validation at the tainted entry point.

Concretely, in keeperapi/src/browser/platform.ts, inside static async decrypt(...), before the switch (encryptionType), introduce validation that checks encryptionType against the allowed set, and throws if it is anything else. The rest of the logic can remain unchanged, so existing functionality for known encryption types is not affected.


Suggested changeset 1
keeperapi/src/browser/platform.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/keeperapi/src/browser/platform.ts b/keeperapi/src/browser/platform.ts
--- a/keeperapi/src/browser/platform.ts
+++ b/keeperapi/src/browser/platform.ts
@@ -432,6 +432,12 @@
         encryptionType: EncryptionType,
         storage?: KeyStorage
     ): Promise<Uint8Array> {
+        // Validate encryptionType to avoid prototype-pollution via unexpected keys
+        const allowedTypes: EncryptionType[] = ['cbc', 'gcm', 'rsa', 'ecc'];
+        if (!allowedTypes.includes(encryptionType)) {
+            throw Error('Unknown encryption type: ' + encryptionType);
+        }
+
         switch (encryptionType) {
             case 'cbc': {
                 const key = await this.loadKey(keyId, encryptionType, storage);
@@ -451,6 +457,7 @@
                 return this.privateDecryptECWebCrypto(data, key);
             }
             default:
+                // This default should now be unreachable due to the validation above.
                 throw Error('Unknown encryption type: ' + encryptionType);
         }
     }
EOF
@@ -432,6 +432,12 @@
encryptionType: EncryptionType,
storage?: KeyStorage
): Promise<Uint8Array> {
// Validate encryptionType to avoid prototype-pollution via unexpected keys
const allowedTypes: EncryptionType[] = ['cbc', 'gcm', 'rsa', 'ecc'];
if (!allowedTypes.includes(encryptionType)) {
throw Error('Unknown encryption type: ' + encryptionType);
}

switch (encryptionType) {
case 'cbc': {
const key = await this.loadKey(keyId, encryptionType, storage);
@@ -451,6 +457,7 @@
return this.privateDecryptECWebCrypto(data, key);
}
default:
// This default should now be unreachable due to the validation above.
throw Error('Unknown encryption type: ' + encryptionType);
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
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.

1 participant