Skip to content

fix: handle undefined author in plugins and remove deprecated Projects API #1777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
artik0din wants to merge 4 commits into from
Closed

fix: handle undefined author in plugins and remove deprecated Projects API #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
27d1cfe
fix(achievements): remove deprecated Projects (classic) API usage
artik0din Dec 31, 2025
8e305d9
fix(habits): handle commits without author property
artik0din Dec 31, 2025
beee10d
fix(activity): handle PushEvent commits without author
artik0din Dec 31, 2025
5b3e924
feat(deploy): add Heroku container deployment support
artik0din Dec 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions Dockerfile.web
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Base image
FROM node:20-bookworm-slim
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🟡 MEDIUM - Node.js base image uses floating patch version
Agent: microservices

Category: quality

Description:
Node.js base image uses 'node:20-bookworm-slim' which is a floating tag that resolves to the latest patch version of Node 20. This can lead to non-deterministic builds.

Suggestion:
Use a specific patch version like 'node:20.11.1-bookworm-slim' instead of 'node:20-bookworm-slim' to ensure consistent, reproducible builds.

Why this matters: Improves code reliability.

Confidence: 75%
Rule: docker_pin_specific_patch_versions_for_node_js
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray


# Copy repository
COPY . /metrics
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🟠 HIGH - Docker cache invalidation: COPY all files before dependency install
Agent: microservices

Category: quality

Description:
COPY . /metrics copies all source files before dependency installation, which will invalidate the Docker layer cache on every code change, triggering rebuilding of expensive layers like apt packages and npm dependencies.

Suggestion:
First copy only dependency manifest files (package.json, package-lock.json) before RUN npm ci, then copy the remaining source files after installation.

Why this matters: Any file change invalidates cached npm install layer.

Confidence: 95%
Rule: docker_copy_before_install
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

WORKDIR /metrics

# Setup
RUN chmod +x /metrics/source/app/action/index.mjs \
# Install latest chrome dev package, fonts to support major charsets and skip chromium download on puppeteer install
# Based on https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md#running-puppeteer-in-docker
&& apt-get update \
&& apt-get install -y wget gnupg ca-certificates libgconf-2-4 \
&& wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add - \
&& sh -c 'echo "deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main" >> /etc/apt/sources.list.d/google.list' \
&& apt-get update \
&& apt-get install -y google-chrome-stable fonts-ipafont-gothic fonts-wqy-zenhei fonts-thai-tlwg fonts-kacst fonts-freefont-ttf libxss1 libx11-xcb1 libxtst6 lsb-release --no-install-recommends \
# Install deno for miscellaneous scripts
&& apt-get install -y curl unzip \
&& curl -fsSL https://deno.land/x/install/install.sh | DENO_INSTALL=/usr/local sh \
# Install ruby to support github licensed gem
&& apt-get install -y ruby-full git g++ cmake pkg-config libssl-dev \
&& gem install licensed \
# Install python for node-gyp
&& apt-get install -y python3 \
# Clean apt/lists
&& rm -rf /var/lib/apt/lists/* \
# Install node modules and rebuild indexes
&& npm ci \
&& npm run build
Comment on lines +5 to +30
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🟡 MEDIUM - Copying entire project directory into production image
Agent: security

Category: security

Description:
Line 5 copies the entire project directory (COPY . /metrics) into the production image. This includes test files, source code, documentation, and git files that are unnecessary at runtime.

Suggestion:
Use .dockerignore to exclude unnecessary files (tests, docs, source, .git, .env files). Consider using multi-stage build where only necessary runtime artifacts are copied.

Why this matters: Smaller images pull faster, have fewer vulnerabilities, and reduce secrets exposure.

Confidence: 80%
Rule: docker_remove_unnecessary_files_from_production
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray


# Environment variables
ENV PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true
ENV PUPPETEER_BROWSER_PATH="google-chrome-stable"
ENV NODE_ENV=production

# Run web server
CMD ["npm", "start"]
Comment on lines +37 to +38
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🟠 HIGH - Missing HEALTHCHECK in Docker container
Agent: bugs

Category: bug

Description:
The Dockerfile does not include a HEALTHCHECK instruction. Without it, Docker orchestrators cannot automatically detect if the application is unhealthy.

Suggestion:
Add a HEALTHCHECK instruction to monitor the application health, e.g., HEALTHCHECK --interval=30s --timeout=5s CMD node -e "require('http').get('http://localhost:3000', (r) => {if (r.statusCode !== 200) throw new Error()})"

Why this matters: Missing healthcheck prevents automatic recovery from application failures.

Confidence: 85%
Rule: docker_missing_healthcheck
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

Comment on lines +1 to +38
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🟠 HIGH - Container runs as root user
Agent: security

Category: security

Description:
The Dockerfile does not specify a USER instruction. Docker containers without an explicit non-root user will run as root (UID 0), increasing blast radius if the container is compromised.

Suggestion:
Add a USER instruction to run the container as a non-root user. Create a dedicated user with RUN useradd -m -u 1000 appuser && USER appuser

Why this matters: Root in container may have elevated privileges on host depending on configuration.

Confidence: 95%
Rule: docker_running_as_root
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

Comment on lines +1 to +38
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🟠 HIGH - Single-stage Docker build leaves build tools in production
Agent: security

Category: security

Description:
The Dockerfile uses a single-stage build pattern. Build dependencies (npm, python3, ruby, g++, cmake, git, deno, curl, unzip) are included in the final production image, increasing attack surface and image size.

Suggestion:
Use multi-stage Docker build with 'AS' aliases. Separate build stage from runtime stage. Only copy necessary runtime artifacts from build stage using 'COPY --from=build'.

Why this matters: Security issues can lead to data breaches.

Confidence: 90%
Rule: docker_implement_multi_stage_builds_for_product
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

1 change: 1 addition & 0 deletions Procfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
web: npm start
3 changes: 3 additions & 0 deletions heroku.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
build:
docker:
web: Dockerfile.web
8 changes: 7 additions & 1 deletion source/app/metrics/setup.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default async function({log = true, sandbox = false, community = {}, extr
authenticated: null,
templates: {},
queries: {},
settings: {port: 3000},
settings: {port: Number(process.env.PORT) || 3000},
metadata: {},
paths: {
statics: __statics,
Expand Down Expand Up @@ -63,6 +63,12 @@ export default async function({log = true, sandbox = false, community = {}, extr
}
}

//Environment variables override (for Heroku/Docker deployment)
if (process.env.METRICS_TOKEN)
conf.settings.token = process.env.METRICS_TOKEN
if (process.env.PORT)
conf.settings.port = Number(process.env.PORT)

if (!conf.settings.templates)
conf.settings.templates = {default: "classic", enabled: []}
if (!conf.settings.plugins)
Copy link
Copy Markdown

@diffray-bot diffray-bot Dec 31, 2025

Choose a reason for hiding this comment

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

🔴 CRITICAL - JSON.parse without try-catch for package.json
Agent: bugs

Category: bug

Description:
JSON.parse() can throw a SyntaxError if package.json contains invalid JSON, but this operation is not wrapped in a try-catch block. The previous try-catch block ended at line 64, leaving this JSON.parse unprotected.

Suggestion:
Wrap the JSON.parse operation in a try-catch block, similar to the pattern used for settings.json in lines 46-64.

Why this matters: Unhandled errors crash the application or hide bugs.

Confidence: 92%
Rule: bug_missing_try_catch
Review ID: 8aeb9742-af0a-4aae-8c5e-a98c17691acc
Rate it 👍 or 👎 to improve future reviews | Powered by diffray

Expand Down
26 changes: 11 additions & 15 deletions source/plugins/achievements/list/users.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,17 @@
})
}

//Manager
{
const value = user.projects.totalCount
const unlock = user.projects.nodes?.shift()

list.push({
title: "Manager",
text: `Created ${value} user project${imports.s(value)}`,
icon:
'<g stroke-width="2" fill="none" fill-rule="evenodd"><path d="M29 16V8.867C29 7.705 29.627 7 30.692 7h18.616C50.373 7 51 7.705 51 8.867v38.266C51 48.295 50.373 49 49.308 49H30.692C29.627 49 29 48.295 29 47.133V39m-4-23V9c0-1.253-.737-2-2-2H7c-1.263 0-2 .747-2 2v34c0 1.253.737 2 2 2h16c1.263 0 2-.747 2-2v-4" stroke="#secondary" stroke-linecap="round"/><path stroke="#secondary" d="M51.557 12.005h-22M5 12.005h21"/><path d="M14 33V22c0-1.246.649-2 1.73-2h28.54c1.081 0 1.73.754 1.73 2v11c0 1.246-.649 2-1.73 2H15.73c-1.081 0-1.73-.754-1.73-2z" stroke="#primary" stroke-linecap="round" stroke-linejoin="round"/><path d="M19 29v-3c0-.508.492-1 1-1h3c.508 0 1 .492 1 1v3c0 .508-.492 1-1 1h-3c-.508-.082-1-.492-1-1z" stroke="#primary"/><path stroke="#primary" stroke-linecap="round" stroke-linejoin="round" d="M28.996 27.998h12M9.065 20.04a7.062 7.062 0 00-.023 1.728m.775 2.517c.264.495.584.954.954 1.369"/></g>',
...rank(value, [1, 2, 3, 4, 5]),
value,
unlock: new Date(unlock?.createdAt),
})
}
//Manager - DISABLED: Projects (classic) has been deprecated by GitHub (May 2024)
//See: https://github.blog/changelog/2024-05-23-sunset-notice-projects-classic/
//{
// const value = user.projects?.totalCount ?? 0

Check failure on line 59 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
// const unlock = user.projects?.nodes?.shift()

Check failure on line 60 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
// list.push({

Check failure on line 61 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
// title: "Manager",

Check failure on line 62 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
// text: `Created ${value} user project${imports.s(value)}`,

Check failure on line 63 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
// ...

Check failure on line 64 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
// })

Check failure on line 65 in source/plugins/achievements/list/users.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
//}

//Reviewer
{
Expand Down
11 changes: 5 additions & 6 deletions source/plugins/achievements/queries/achievements.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@ query AchievementsDefault {
totalCount
}
}
projects(first: 1, orderBy: {field: CREATED_AT, direction: ASC}) {
totalCount
#nodes { This requires additional scopes :/
# name
#}
}
# Projects (classic) has been deprecated by GitHub (May 2024)
# See: https://github.blog/changelog/2024-05-23-sunset-notice-projects-classic/
# projects(first: 1, orderBy: {field: CREATED_AT, direction: ASC}) {
# totalCount
# }
packages(first: 1, orderBy: {direction: ASC, field: CREATED_AT}) {
totalCount
#nodes { This requires additional scopes :/
Expand Down
2 changes: 1 addition & 1 deletion source/plugins/activity/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export default async function({login, data, rest, q, account, imports}, {enabled
//Pushed commits
case "PushEvent": {
let {size, commits, ref} = payload
commits = commits.filter(({author: {email}}) => imports.filters.text(email, ignored))
commits = commits.filter(commit => commit.author && imports.filters.text(commit.author.email, ignored))
if (!commits.length)
return null
if (commits.slice(-1).pop()?.message.startsWith("Merge branch "))
Expand Down
1 change: 1 addition & 0 deletions source/plugins/habits/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
...await Promise.allSettled(
commits
.flatMap(({payload}) => payload.commits)
.filter(commit => commit && commit.author) // Filter out commits without author

Check failure on line 51 in source/plugins/habits/index.mjs

View workflow job for this annotation

GitHub Actions / Lint code 

Unexpected space or tab after '//' in comment
.filter(({author}) => data.shared["commits.authoring"].filter(authoring => author?.login?.toLocaleLowerCase().includes(authoring) || author?.email?.toLocaleLowerCase().includes(authoring) || author?.name?.toLocaleLowerCase().includes(authoring)).length)
.map(async commit => (await rest.request(commit)).data.files),
),
Expand Down
Loading