Merge 1.1.1.3 Release into master #33
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # Dependencies | ||
| node_modules | ||
| npm-debug.log* | ||
| package-lock.json | ||
|
|
||
| # Testing | ||
| coverage | ||
| *.test.js | ||
| test | ||
|
|
||
| # Documentation | ||
| *.md | ||
| !README.md | ||
|
|
||
| # Git | ||
| .git | ||
| .gitignore | ||
|
|
||
| # IDE | ||
| .vscode | ||
| .idea | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # Logs | ||
| logs | ||
| *.log | ||
|
|
||
| # Environment | ||
| .env | ||
| .env.local | ||
| .env.*.local | ||
|
|
||
| # OS | ||
| .DS_Store | ||
| Thumbs.db | ||
|
|
||
| # Build artifacts | ||
| dist | ||
| build |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| name: Test | ||
| on: | ||
| workflow_dispatch: | ||
| push: | ||
| branches: | ||
| - "*.*.*" | ||
| - "main" | ||
| pull_request: | ||
| branches: | ||
| - "*" | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| test: | ||
| name: Test Changes | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout Repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| submodules: 'recursive' | ||
|
Comment on lines
+18
to
+21
There was a problem hiding this comment. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow ---'
cat -n .github/workflows/test.yml
printf '%s\n' '--- package manifests ---'
git ls-files | grep -E '(^|/)(package\.json|npm-shrinkwrap\.json|package-lock\.json|yarn\.lock|pnpm-lock\.yaml)$' || true
printf '%s\n' '--- lifecycle-related scripts and git configuration references ---'
rg -n --hidden -S 'persist-credentials|npm (install|ci)|preinstall|install|postinstall|prepare|git config|credential' \
.github package.json package-lock.json npm-shrinkwrap.json 2>/dev/null || trueRepository: CBIIT/crdc-ctdc-authn Length of output: 2796 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- package.json ---'
cat -n package.json
printf '%s\n' '--- submodule configuration ---'
if [ -f .gitmodules ]; then
cat -n .gitmodules
else
printf '%s\n' 'No .gitmodules file'
fi
printf '%s\n' '--- repository Git metadata tracked by the worktree ---'
git ls-files -s | awk '$1 == "160000" {print}'Repository: CBIIT/crdc-ctdc-authn Length of output: 2313 🌐 Web query:
💡 Result: The persist-credentials input for actions/checkout (including v4) has a default value of true [1][2][3]. When set to true, the action configures the repository to use the provided authentication token (typically the GITHUB_TOKEN) for subsequent git operations [4][3][5]. Regarding how it interacts with the local git configuration: - In versions prior to the changes introduced in later releases (like v6.0.0+), the authentication token was written directly into the local.git/config file [5]. - Starting with more recent versions (e.g., v6.0.0 and later), the action improved security by storing the credentials in a separate file within the $RUNNER_TEMP directory, using an includeIf directive in the local.git/config to reference it, rather than placing the token directly in the repository's configuration [6][7][5]. In all versions, the token is automatically removed during the post-job cleanup process [4][3]. If you wish to disable this behavior and prevent the action from configuring any credentials, you can set persist-credentials to false [4][3][5]. Citations:
Disable checkout credential persistence.
🧰 Tools🪛 zizmor (1.29.0)[warning] 18-21: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false (artipacked) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| - name: Setup Node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "24.x" | ||
| cache: "npm" | ||
|
|
||
| - name: Install Dependencies | ||
| run: npm install | ||
|
|
||
| - name: Run Tests | ||
| env: | ||
| DATABASE_TYPE: mysql | ||
| MYSQL_HOST: localhost | ||
| MYSQL_PORT: 3306 | ||
| MYSQL_USER: test | ||
| MYSQL_PASSWORD: test | ||
| MYSQL_DATABASE: test | ||
| COOKIE_SECRET: test-secret | ||
| TOKEN_SECRET: test-token-secret | ||
| run: npm run test:ci | ||
|
|
||
| # Upload test coverage reports to Coveralls | ||
| - name: Coveralls GitHub Action | ||
| uses: coverallsapp/github-action@v2 | ||
| if: always() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,7 @@ sessions/ | |
| logs/ | ||
| yaml/ | ||
| newrelic_agent.log | ||
| .env | ||
| /coverage | ||
| .vscode/ | ||
| .DS_Store | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 24 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| FROM node:24-alpine3.23 AS fnl_base_image | ||
| ENV PORT=8082 | ||
| ENV NODE_ENV=production | ||
| WORKDIR /usr/src/app | ||
|
|
||
| # Upgrade npm to latest version to fix bundled vulnerabilities | ||
| RUN npm install -g npm@11.14.1 | ||
|
|
||
| COPY package.json ./ | ||
| # Use npm install instead of npm ci to apply npm overrides for transitive dependency CVE fixes | ||
| RUN npm install --omit=dev --ignore-scripts | ||
|
Comment on lines
+9
to
+11
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- tracked files ---'
git ls-files -- Dockerfile .dockerignore package.json package-lock.json npm-shrinkwrap.json
printf '%s\n' '--- Dockerfile ---'
cat -n Dockerfile
printf '%s\n' '--- .dockerignore ---'
cat -n .dockerignore
printf '%s\n' '--- package manifests ---'
for f in package.json package-lock.json npm-shrinkwrap.json; do
if [ -f "$f" ]; then
printf '%s\n' "--- $f ---"
if [ "$f" = package.json ]; then
cat -n "$f"
else
sed -n '1,80p' "$f"
fi
fi
done
printf '%s\n' '--- npm install/ci references ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' 'npm (install|ci)|package-lock\.json|npm-shrinkwrap\.json|overrides' .Repository: CBIIT/crdc-ctdc-authn Length of output: 6462 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
import json
from pathlib import Path
lock = json.loads(Path("package-lock.json").read_text())
root = lock.get("packages", {}).get("", {})
print("lockfileVersion:", lock.get("lockfileVersion"))
print("root has overrides:", "overrides" in root)
print("manifest overrides:", json.loads(Path("package.json").read_text()).get("overrides"))
for name in ("brace-expansion", "picomatch", "ip-address"):
entries = []
prefix = "node_modules/" + name
for path, meta in lock.get("packages", {}).items():
if path == prefix or path.endswith("/" + prefix):
entries.append((path, meta.get("version"), meta.get("dev"), meta.get("resolved")))
print(name, entries)
print("top-level dependency entries:")
for name in ("brace-expansion", "picomatch", "ip-address"):
print(name, lock.get("dependencies", {}).get(name))
PY
printf '%s\n' '--- relevant lockfile entries and override metadata ---'
rg -n -C 3 '"(brace-expansion|picomatch|ip-address|overrides)"' package-lock.json | head -240
printf '%s\n' '--- repository npm and lockfile metadata ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' \
'npm@|packageManager|lockfileVersion|npm install|npm ci|overrides' \
.github Dockerfile package.json package-lock.json README.md 2>/dev/null || trueRepository: CBIIT/crdc-ctdc-authn Length of output: 6403 🌐 Web query:
💡 Result: The npm ci command is designed to perform a strict, reproducible installation of dependencies based exclusively on the existing package-lock.json or npm-shrinkwrap.json file [1][2]. Because it treats the lockfile as the source of truth, it does not dynamically recalculate the dependency tree, nor does it apply changes from the overrides field in package.json at runtime [3]. Key behavior regarding overrides and npm ci: 1. Overrides are not dynamic: The overrides field in package.json is used by npm install to resolve the dependency tree and generate the package-lock.json [4]. Once the lockfile is generated, the overrides are effectively baked into the resolved versions recorded in that lockfile. 2. npm ci ignores local package.json overrides: If you modify the overrides field in your package.json but do not regenerate your package-lock.json (using npm install), npm ci will detect a discrepancy between the intended state (in package.json) and the locked state (in the lockfile) [1][2]. 3. Synchronization requirement: npm ci strictly enforces synchronization between package.json and the lockfile [1][5]. If your overrides result in a dependency tree that does not match the information present in your current package-lock.json, npm ci will exit with an error, requiring you to run npm install to update the lockfile before continuing [1][6]. To ensure overrides are applied correctly in your CI/CD environment, you must: - Run npm install locally after changing any overrides to regenerate and update the package-lock.json file [3]. - Commit the updated package-lock.json to your version control system [3]. - Ensure your CI environment uses a version of npm that supports overrides (npm v8.3.0 or later) [3]. If your lockfile is not updated to reflect the overrides, npm ci will either fail due to a sync error or proceed using the stale, pre-override versions stored in the lockfile [3]. Citations:
Keep the lockfile in the production build path. Remove 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: MCP tools |
||
|
|
||
| # Copy application files | ||
| COPY --chown=node:node . . | ||
|
Comment on lines
+13
to
14
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- Dockerfile ---'
cat -n Dockerfile
printf '%s\n' '--- repository files relevant to runtime, permissions, and startup ---'
git ls-files | rg '(^|/)(Dockerfile|docker-compose[^/]*|package\.json|pnpm-lock\.yaml|yarn\.lock|package-lock\.json|.*entrypoint.*|.*start.*|.*log.*)$' || true
printf '%s\n' '--- Dockerfile directives ---'
rg -n '^(FROM|RUN|COPY|ADD|WORKDIR|USER|EXPOSE|ENTRYPOINT|CMD)\b|log|8082|node' DockerfileRepository: CBIIT/crdc-ctdc-authn Length of output: 1183 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- package scripts and runtime dependencies ---'
cat package.json
printf '%s\n' '--- startup and filesystem-write behavior ---'
for f in bin/www app.js server.js index.js; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f"
fi
done
rg -n -i 'writeFile|appendFile|createWriteStream|mkdir|mkdirSync|fs\.|/var/log|log/|logs/|bento-event-logging|process\.cwd|tmp|8082' \
--glob '!package-lock.json' --glob '!node_modules/**' . || true
printf '%s\n' '--- tracked directories and files near runtime ---'
git ls-files | sed -n '1,160p'Repository: CBIIT/crdc-ctdc-authn Length of output: 8512 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- ignore rules affecting logs and the build context ---'
cat -n .dockerignore
printf '\n--- git ignore rules affecting logs ---\n'
cat -n .gitignore
printf '\n--- filesystem writes and relative paths in application sources ---\n'
rg -n -C 2 'fs\.|writeFile|appendFile|createWriteStream|mkdir|logs|process\.cwd|__dirname' \
app.js bin services routes controllers neo4j idps config.js 2>/dev/null || true
printf '\n--- Dockerfile ownership-sensitive directives ---\n'
awk '
/^(FROM|WORKDIR|RUN|COPY|ADD|USER|CMD|ENTRYPOINT)/ { print NR ":" $0 }
' DockerfileRepository: CBIIT/crdc-ctdc-authn Length of output: 2631 🌐 Web query:
💡 Result: In Dockerfiles, the USER instruction does not influence the ownership of files created or copied by subsequent COPY, ADD, or WORKDIR instructions [1][2][3]. By default, these operations create files and directories owned by the root user (UID 0, GID 0), regardless of the current USER setting [4][5][3]. For the COPY instruction, you must explicitly use the --chown flag to set non-root ownership during the copy process [4][2]. The flag supports either username and groupname strings (which are looked up in the container's /etc/passwd and /etc/group files) or direct integer UID and GID values [4][6]. Regarding WORKDIR, while it automatically creates a directory if it does not exist, that directory is typically created as root [7][1][8]. Depending on your builder version (e.g., BuildKit), there have been variations in behavior, but the established best practice is to explicitly create the directory with the desired permissions and ownership using a RUN instruction before switching to a non-root USER [7][1]. Commonly recommended patterns include: 1. Creating the non-root user and setting up directory ownership using RUN instructions while still as root. 2. Using --chown during COPY if you need specific ownership for files. 3. Keeping the USER instruction near the end of your Dockerfile to ensure that administrative tasks like installing dependencies or setting up file permissions are performed with root privileges [9]. top_results: [4][7][1][2][3][9] Citations:
Run the application as Add 🤖 Prompt for AI AgentsSource: MCP tools |
||
|
|
||
| EXPOSE 8082 | ||
|
|
||
| CMD [ "node", "./bin/www" ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| var createError = require('http-errors'); | ||
| var express = require('express'); | ||
| var path = require('path'); | ||
|
|
@@ -13,14 +12,15 @@ const cookieParser = require('cookie-parser'); | |
| console.log(config); | ||
|
|
||
| const LOG_FOLDER = 'logs'; | ||
| let accessLogStream = process.stdout; | ||
| try { | ||
| fs.mkdirSync(LOG_FOLDER, { recursive: true }); | ||
| accessLogStream = fs.createWriteStream(path.join(__dirname, LOG_FOLDER, 'access.log'), { flags: 'a' }); | ||
| } catch (err) { | ||
| logger.warn(`Unable to create log directory "${LOG_FOLDER}", falling back to stdout: ${err.message}`); | ||
|
Comment on lines
+15
to
+20
There was a problem hiding this comment. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -t f 'app\.js$|package\.json$' . | head -80
printf '%s\n' '--- app.js outline ---'
if [ -f app.js ]; then
ast-grep outline app.js
printf '%s\n' '--- app.js lines 1-100 ---'
cat -n app.js | sed -n '1,100p'
else
file="$(fd -t f 'app\.js$' . | head -1)"
printf 'selected=%s\n' "$file"
ast-grep outline "$file"
cat -n "$file" | sed -n '1,120p'
fi
printf '%s\n' '--- accessLogStream references ---'
rg -n -C 4 'accessLogStream|createWriteStream|morgan|LOG_FOLDER' . -g '!node_modules' -g '!dist' -g '!build' || trueRepository: CBIIT/crdc-ctdc-authn Length of output: 8007 🏁 Script executed: #!/bin/bash
set +e
printf '%s\n' '--- asynchronous createWriteStream error is outside try/catch ---'
node - <<'JS'
const fs = require('fs');
const target = '/tmp/coderabbit-nonexistent-parent-verify/access.log';
let caught = false;
try {
const stream = fs.createWriteStream(target, { flags: 'a' });
stream.once('error', (err) => {
console.log(`event=${err.code}; caught=${caught}; syncWritable=${stream.writable}`);
process.exit(0);
});
caught = true;
} catch (err) {
console.log(`caught=${err.code}`);
process.exit(2);
}
setTimeout(() => {
console.log('no-error-event');
process.exit(3);
}, 500);
JS
printf 'probe_status=%s\n' "$?"
printf '%s\n' '--- unhandled stream error process behavior ---'
node - <<'JS'
const fs = require('fs');
const stream = fs.createWriteStream('/tmp/coderabbit-nonexistent-parent-verify/access.log', { flags: 'a' });
setTimeout(() => console.log('still-running'), 300);
JS
printf 'unhandled_probe_status=%s\n' "$?"
printf '%s\n' '--- installed/runtime versions ---'
node --versionRepository: CBIIT/crdc-ctdc-authn Length of output: 960 🌐 Web query:
💡 Result: In Node.js, fs.createWriteStream initiates an asynchronous file opening process [1][2]. Because this process is asynchronous, errors encountered during the opening phase (such as ENOENT or EISDIR) are emitted as 'error' events on the stream instance itself [3][4]. Key points regarding this behavior and morgan integration: Asynchronous Error Handling: Since fs.createWriteStream does not block the event loop, errors are not thrown synchronously at the moment of the function call [1][2]. You must attach an 'error' event listener to the returned stream immediately to catch these errors [3][4]. Morgan Stream Integration: When using morgan with a file stream (e.g., app.use(morgan('common', { stream: fs.createWriteStream(...) }))), morgan simply writes to the provided stream [5][6]. It does not inherently handle errors emitted by that stream [7]. If the underlying stream fails (e.g., the log file directory does not exist or permissions are denied), the error event will be emitted on the stream object itself [3][4]. Best Practices for Error Handling: 1. Attach Listeners: Always attach an 'error' listener to your custom log streams [3]. 2. Directory Existence: Ensure the target directory for your log file exists before calling fs.createWriteStream, as the stream will not create the directory for you and will emit an ENOENT error if the path is invalid [4]. 3. Stream Lifecycle: By default, if autoClose is true (the default), the file descriptor will be closed automatically on 'error' or 'finish' [8][2]. 4. Alternative Log Management: For production environments, it is often recommended to use dedicated logging libraries (like Winston or Pino) which provide more robust built-in mechanisms for stream management, rotation, and error handling compared to passing a raw fs.createWriteStream to morgan [9]. Citations:
Handle
🤖 Prompt for AI AgentsSource: MCP tools
Comment on lines
+19
to
+20
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 '\b(logger|morgan)\b' app.jsRepository: CBIIT/crdc-ctdc-authn Length of output: 1189 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- app.js ---'
sed -n '1,35p' app.js
printf '%s\n' '--- morgan declarations ---'
rg -n -C 3 '"morgan"|require\(['"'"']morgan['"'"']\)|logger\.(warn|info|error)' package.json package-lock.json npm-shrinkwrap.json yarn.lock pnpm-lock.yaml app.js 2>/dev/null || trueRepository: CBIIT/crdc-ctdc-authn Length of output: 3236 Replace
🤖 Prompt for AI AgentsSource: MCP tools |
||
| } | ||
|
|
||
|
|
||
| var authRouter = require('./routes/auth'); | ||
| var app = express(); | ||
| app.use(cors()); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Start the MySQL service for this job.
Lines 34-39 configure the application to use MySQL at
localhost:3306, but this workflow does not start MySQL. Environment variables do not provision a database. Add a MySQL job service, map port3306, and wait for its health check beforenpm run test:ci.Proposed fix
jobs: test: name: Test Changes runs-on: ubuntu-latest + services: + mysql: + image: mysql:8.0 + env: + MYSQL_DATABASE: test + MYSQL_USER: test + MYSQL_PASSWORD: test + MYSQL_ROOT_PASSWORD: test-root-password + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping -h localhost -u test -ptest" + --health-interval=10s + --health-timeout=5s + --health-retries=5Also applies to: 32-42
🤖 Prompt for AI Agents