-
Notifications
You must be signed in to change notification settings - Fork 0
fix: resolve upcoming.ts mixed currency total, prevent SQL injection, and fix scanner table names #29
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.
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
fix: resolve upcoming.ts mixed currency total, prevent SQL injection, and fix scanner table names #29
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { test, expect, beforeEach, afterEach, beforeAll } from "vitest" | ||
| import { consola } from "consola" | ||
| import initSqlJs from "sql.js" | ||
| import type { Database } from "sql.js" | ||
|
|
||
| const logMessages: string[] = [] | ||
| const infoMessages: string[] = [] | ||
|
|
||
| let testDb: Database | ||
|
|
||
| beforeAll(async () => { | ||
| const SQL = await initSqlJs() | ||
| testDb = new SQL.Database() | ||
| testDb.run("PRAGMA foreign_keys = ON") | ||
| testDb.run(`CREATE TABLE IF NOT EXISTS subscriptions ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| name TEXT NOT NULL, | ||
| price INTEGER NOT NULL, | ||
| currency TEXT NOT NULL, | ||
| cycle TEXT NOT NULL, | ||
| status TEXT NOT NULL DEFAULT 'active', | ||
| billing_day INTEGER, | ||
| created_at TEXT NOT NULL DEFAULT (date('now')) | ||
| )`) | ||
| testDb.run(`CREATE TABLE IF NOT EXISTS tags ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| name TEXT NOT NULL UNIQUE | ||
| )`) | ||
| testDb.run(`CREATE TABLE IF NOT EXISTS subscription_tags ( | ||
| subscription_id INTEGER NOT NULL, | ||
| tag_id INTEGER NOT NULL, | ||
| PRIMARY KEY (subscription_id, tag_id), | ||
| FOREIGN KEY (subscription_id) REFERENCES subscriptions(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (tag_id) REFERENCES tags(id) ON DELETE CASCADE | ||
| )`) | ||
| testDb.run(`CREATE TABLE IF NOT EXISTS price_history ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| subscription_id INTEGER NOT NULL, | ||
| old_price INTEGER NOT NULL, | ||
| new_price INTEGER NOT NULL, | ||
| changed_at TEXT NOT NULL DEFAULT (date('now')), | ||
| FOREIGN KEY (subscription_id) REFERENCES subscriptions(id) ON DELETE CASCADE | ||
| )`) | ||
|
|
||
| const db = await import("../db.ts") | ||
| db.__setDb(testDb) | ||
| }) | ||
|
|
||
| beforeEach(() => { | ||
| testDb.run("DELETE FROM subscription_tags") | ||
| testDb.run("DELETE FROM tags") | ||
| testDb.run("DELETE FROM subscriptions") | ||
|
|
||
| logMessages.length = 0 | ||
| infoMessages.length = 0 | ||
|
|
||
| const stripAnsi = (s: string) => s.replace(/\x1b\[[0-9;]*m/g, "") | ||
|
|
||
| consola.mockTypes((_type: string, _defaults: object) => { | ||
| return (...args: unknown[]) => { | ||
| const str = args.map((a) => String(a)).join(" ") | ||
| const clean = stripAnsi(str) | ||
| if (_type === "log") logMessages.push(clean) | ||
| if (_type === "info") infoMessages.push(clean) | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| consola.mockTypes() | ||
| }) | ||
|
|
||
| test("showUpcoming shows info when no subscriptions", async () => { | ||
| const { showUpcoming } = await import("../upcoming.ts") | ||
| showUpcoming(7) | ||
| expect(infoMessages.some((m) => m.includes("No active subscriptions"))).toBe(true) | ||
| }) | ||
|
|
||
| test("showUpcoming shows info when no upcoming bills", async () => { | ||
| const db = await import("../db.ts") | ||
| // Create a subscription with billing far in the future | ||
| db.writeSubscription({ name: "Yearly", price: 1000, currency: "USD", cycle: "yearly", tags: [], status: "active", createdAt: "2025-01-01", billingDay: 1 }) | ||
|
|
||
| const { showUpcoming } = await import("../upcoming.ts") | ||
| showUpcoming(7) | ||
| expect(infoMessages.some((m) => m.includes("No upcoming bills"))).toBe(true) | ||
| }) | ||
|
|
||
| test("showUpcoming shows upcoming monthly subscription", async () => { | ||
| const db = await import("../db.ts") | ||
| // Create a subscription with billing day = 25 (tomorrow-ish) | ||
| const today = new Date() | ||
| const billingDay = today.getDate() + 1 > 28 ? 28 : today.getDate() + 1 | ||
| db.writeSubscription({ name: "Netflix", price: 1500, currency: "JPY", cycle: "monthly", tags: ["video"], status: "active", billingDay, createdAt: "2026-01-15" }) | ||
|
|
||
| const { showUpcoming } = await import("../upcoming.ts") | ||
| showUpcoming(30) | ||
| expect(logMessages.length).toBeGreaterThan(0) | ||
| const output = logMessages.join("\n") | ||
| expect(output).toContain("Netflix") | ||
| expect(output).toContain("¥1,500") | ||
| }) | ||
|
|
||
| test("showUpcoming excludes cancelled subscriptions", async () => { | ||
| const db = await import("../db.ts") | ||
| db.writeSubscription({ name: "Active", price: 100, currency: "USD", cycle: "monthly", tags: [], status: "active", billingDay: 28, createdAt: "2026-01-01" }) | ||
| db.writeSubscription({ name: "Cancelled", price: 200, currency: "USD", cycle: "monthly", tags: [], status: "cancelled", billingDay: 28, createdAt: "2026-01-01" }) | ||
|
|
||
| const { showUpcoming } = await import("../upcoming.ts") | ||
| showUpcoming(30) | ||
| const output = logMessages.join("\n") | ||
| expect(output).toContain("Active") | ||
| expect(output).not.toContain("Cancelled") | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { consola } from "consola" | ||
| import pc from "picocolors" | ||
| import type { SharedArgs } from "./types.ts" | ||
| import { getSubscriptions } from "./db.ts" | ||
| import { formatPrice } from "./display.ts" | ||
| import { calcSummary } from "./payment.ts" | ||
| import { loadConfig } from "./config.ts" | ||
| import { periodFactor } from "./types.ts" | ||
|
|
||
| export function showAnalytics(): void { | ||
| const list = getSubscriptions().filter((s) => s.status !== "cancelled") | ||
| if (list.length === 0) { | ||
| consola.info("No active subscriptions found") | ||
| return | ||
| } | ||
|
|
||
| const config = loadConfig() | ||
| const data = calcSummary(list) | ||
|
|
||
| // Header | ||
| consola.log(pc.bold("📊 Subscription Analytics")) | ||
| consola.log("") | ||
|
|
||
| // Overview | ||
| consola.log(pc.bold("Overview:")) | ||
| consola.log(` Total subscriptions: ${pc.bold(String(data.totalCount))}`) | ||
| consola.log(` Status breakdown:`) | ||
| const activeCount = list.filter((s) => s.status === "active").length | ||
| const pausedCount = list.filter((s) => s.status === "paused").length | ||
| const cancelledCount = getSubscriptions().filter((s) => s.status === "cancelled").length | ||
| consola.log(` ${pc.green(`active: ${activeCount}`)}`) | ||
| if (pausedCount > 0) consola.log(` ${pc.yellow(`paused: ${pausedCount}`)}`) | ||
| if (cancelledCount > 0) consola.log(` ${pc.red(`cancelled: ${cancelledCount}`)}`) | ||
|
Comment on lines
+11
to
+33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Fix the Line 11 (and Lines 28-30) reads 🧰 Tools🪛 GitHub Actions: app-ci / 1_check _ check.txt[error] 11-11: TypeScript (tsc --noEmit) failed: TS2339 Property 'status' does not exist on type 'SharedArgs'. 🪛 GitHub Actions: app-ci / check _ check[error] 11-11: TypeScript (tsc --noEmit) error TS2339: Property 'status' does not exist on type 'SharedArgs'. 🤖 Prompt for AI AgentsSource: Pipeline failures |
||
|
|
||
| if (data.mostExpensive) { | ||
| const me = data.mostExpensive | ||
| consola.log(` Most expensive: ${pc.bold(me.name)} (${formatPrice(me.price, me.currency)}/${me.cycle})`) | ||
| } | ||
|
|
||
| // Monthly spending | ||
| consola.log("") | ||
| consola.log(pc.bold("Monthly spending:")) | ||
| for (const [ccy, total] of Object.entries(data.monthlyByCurrency).sort()) { | ||
| consola.log(` ${ccy} ${formatPrice(Math.round(total), ccy)}`) | ||
| } | ||
|
|
||
| // Budget | ||
| if (config.monthlyBudget > 0) { | ||
| const defaultCurrency = config.defaultCurrency || "USD" | ||
| // Sum all monthly costs in their original currencies | ||
| const monthlyTotal = list.reduce((sum, sub) => sum + sub.price * periodFactor(sub.cycle, "monthly"), 0) | ||
| // For budget display, use default currency as reference | ||
| const budgetDisplay = formatPrice(config.monthlyBudget, defaultCurrency) | ||
| const spentDisplay = formatPrice(monthlyTotal, "USD") | ||
| consola.log(` ${pc.dim("─".repeat(30))}`) | ||
| consola.log(` Budget: ${pc.bold(budgetDisplay)}`) | ||
| const remaining = config.monthlyBudget - monthlyTotal | ||
| const remainingDisplay = formatPrice(remaining, "USD") | ||
|
Comment on lines
+48
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Budget/tag totals are mixing currencies and hardcoding USD output. Line 51 aggregates monthly totals across all currencies, then Lines 54/58/73 format as Also applies to: 67-74 🤖 Prompt for AI Agents |
||
| if (remaining >= 0) { | ||
| consola.log(` Remaining: ${pc.green(remainingDisplay)}`) | ||
| } else { | ||
| consola.log(` Over budget: ${pc.red(remainingDisplay.replace("-", ""))}`) | ||
| } | ||
| } | ||
|
|
||
| // Tags breakdown | ||
| if (Object.keys(data.monthlyByTag).length > 0) { | ||
| consola.log("") | ||
| consola.log(pc.bold("Monthly by tag:")) | ||
| const sorted = Object.entries(data.monthlyByTag).sort((a, b) => b[1].monthly - a[1].monthly) | ||
| for (const [tag, info] of sorted) { | ||
| consola.log( | ||
| ` ${tag.padEnd(16)} ${formatPrice(Math.round(info.monthly), "USD")}/month (${info.count} sub${info.count > 1 ? "s" : ""})`, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import { readFileSync, writeFileSync, mkdirSync, existsSync } from "node:fs" | ||
| import { homedir } from "node:os" | ||
| import path from "node:path" | ||
| import { consola } from "consola" | ||
| import { safeJsonParse } from "./safe-json.ts" | ||
| import type { SubtrackConfig } from "./types.ts" | ||
|
|
||
| export const CONFIG_KEYS = [ | ||
| "defaultCurrency", | ||
| "monthlyBudget", | ||
| "theme", | ||
| ] as const | ||
|
|
||
| export type ConfigKey = (typeof CONFIG_KEYS)[number] | ||
|
|
||
| const DEFAULT_CONFIG: SubtrackConfig = { | ||
| defaultCurrency: "USD", | ||
| monthlyBudget: 0, | ||
| theme: "default", | ||
| } | ||
|
|
||
| function getConfigDir(): string { | ||
| return process.env.SUBSC_CLI_DB_DIR ?? path.join(homedir(), ".config", "subtrack") | ||
| } | ||
|
|
||
| function getConfigPath(): string { | ||
| return path.join(getConfigDir(), "config.json") | ||
| } | ||
|
|
||
| let _config: SubtrackConfig | null = null | ||
|
|
||
| export function loadConfig(): SubtrackConfig { | ||
| if (_config) return _config | ||
|
|
||
| const configPath = getConfigPath() | ||
| if (existsSync(configPath)) { | ||
| try { | ||
| const raw = readFileSync(configPath, "utf-8") | ||
| const parsed = safeJsonParse<Partial<SubtrackConfig>>(raw) | ||
| _config = { ...DEFAULT_CONFIG, ...parsed } | ||
| return _config | ||
| } catch { | ||
| // corrupt config — use defaults | ||
| } | ||
| } | ||
|
|
||
| _config = { ...DEFAULT_CONFIG } | ||
| return _config | ||
| } | ||
|
|
||
| export function resetConfig(): void { | ||
| _config = null | ||
| } | ||
|
|
||
| export function setConfig(key: ConfigKey, value: string): boolean { | ||
| const config = loadConfig() | ||
|
|
||
| switch (key) { | ||
| case "defaultCurrency": { | ||
| if (!/^[A-Z]{3}$/.test(value)) { | ||
| consola.error(`Invalid currency code: "${value}"`) | ||
| return false | ||
| } | ||
| config.defaultCurrency = value | ||
| break | ||
| } | ||
| case "monthlyBudget": { | ||
| const num = Number(value) | ||
| if (isNaN(num) || num < 0) { | ||
| consola.error("monthlyBudget must be a non-negative number") | ||
| return false | ||
| } | ||
| config.monthlyBudget = num | ||
| break | ||
| } | ||
| case "theme": | ||
| config.theme = value | ||
| break | ||
| default: | ||
| consola.error(`Unknown config key: "${key}"`) | ||
| return false | ||
| } | ||
|
|
||
| saveConfig(config) | ||
| consola.success(`Set ${key} = ${value}`) | ||
| return true | ||
| } | ||
|
|
||
| function saveConfig(config: SubtrackConfig): void { | ||
| const configPath = getConfigPath() | ||
| const dir = path.dirname(configPath) | ||
| if (!existsSync(dir)) { | ||
| mkdirSync(dir, { recursive: true, mode: 0o700 }) | ||
| } | ||
| writeFileSync(configPath, JSON.stringify(config, null, 2) + "\n", { mode: 0o600 }) | ||
| _config = config | ||
| } |
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.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
Repository: nazozokc/subtrack
Length of output: 155
Freeze time in these tests to prevent flakiness.
The assertions in "no upcoming bills" and "upcoming monthly subscription" rely on
new Date(), causing instability near billing thresholds. Implementjest.useFakeTimers()with a fixed date inbeforeEachandjest.useRealTimers()inafterEachto ensure deterministic execution.🤖 Prompt for AI Agents