Merge actual server into actual #3
Conversation
…ints (#166) Exposes IBAN in the API which is required for actualbudget#766 to work reliably.
When working on something else, I noticed that `.test.js` files were not running due to `jest.config.json` not including them. I went ahead and re-enabled these tests to make sure that unit tests are actually being run.
…lbudget#141) Users in actualbudget#99 report that Actual in Docker runs on armv7 platforms, although a bit sluggish. I confirmed that the base images for Debian and Alpine support the linux/arm/v7 target and have added them to the platform list in the GitHub Actions workflow. At least one user confirms it works with the bullseye default `Dockerfile`, but before merging it would be great if someone can confirm it works with the Dockerfile.alpine image: ``` git clone https://github.com/jamesmortensen/actual-server.git cd actual-server git checkout armv7-image docker build -t actual-server -f Dockerfile.alpine . docker run -p 5006:5006 actual-server ``` --------- Co-authored-by: Jed Fox <[email protected]>
Update all Docker Hub references to new `actualbudget` organization from `jlongster` personal account. We're officially an org now! A bit of markdown/yaml auto-formatting snuck in, too. Closes actualbudget#364 Corresponding update for the docs site in actualbudget/docs#144 Simultaneous to merging, we need to update our `DOCKER_HUB_*` GitHub secrets in this repo. --------- Co-authored-by: Jed Fox <[email protected]>
This will help people sort out configuration issues. Will open a PR to the docs as well to guide people to troubleshoot using this!
GitHub CI log: ``` [linux/arm/v6 base 6/8] RUN yarn workspaces focus --all --production 204.6 ➤ YN0007: │ bcrypt@npm:5.1.0 must be built because it never has been before or the last one failed 204.6 ➤ YN0007: │ better-sqlite3@npm:8.2.0 must be built because it never has been before or the last one failed ... [linux/arm/v7 base 6/8] RUN yarn workspaces focus --all --production 203.8 ➤ YN0007: │ bcrypt@npm:5.1.0 must be built because it never has been before or the last one failed 203.8 ➤ YN0007: │ better-sqlite3@npm:8.2.0 must be built because it never has been before or the last one failed ``` It seems that both armv6 and armv7 have the same issues with `bcrypt` and `better-sqlite3` not being built. These packages are required to build from source, luckily QEMU use armv7l for compiling. Tested and working on RPi Zero W. --------- Co-authored-by: Jed Fox <[email protected]>
Seems like we already added `tsc` to build the project, but we use the wrong babel preset (Flow) instead of the specific TS one. This is only used in testing to make Jest work (from what I can tell).
CodeQL keeps yelling at us about this… I’m not sure if the filter is smart enough to use this rate limit middleware to remove the warnings, but at least we will be setting a reasonable bound on attempts to crack the server password.
Co-authored-by: Matiss Janis Aboltins <[email protected]>
…192) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…(#190) Helps with actualbudget#919 by adding the `all` field wit both pending and booked transactions to the output of `getTransactionsWithBalance()` and, by extension, the `/nordigen/transactions` endpoint. I could alter the `getTransactions()` to return the `all` field as well but I figured that keeping it such that it returns the output from Nordigen API 1:1 might be better so I left it as is. If you don't agree, let me know and I'll update this.
Co-authored-by: Henrik Maaland <[email protected]>
A small fix: returning JSON response instead of plain-text. The frontend always expects a JSON response. So a tiny fix here..
Previously, the latest artifact list was requested unauthenticated using `ADD "https://api.github.com/..." /tmp/artifacts.json`. While this works locally, on GitHub’s servers it seems that the per-IP rate limit was exceeded. There isn’t a way to get Docker to pass the `Authorization` header that I know of, so this work has been moved to an external shell script that pulls down the relevant data.
Web: actualbudget#1087 Server: actualbudget/actual-server#207 Docs: actualbudget/docs#179 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Remove the bulk in favour of links to our core docs. --------- Co-authored-by: Jed Fox <[email protected]>
…et#527) * Added command lines to enable/disable openid * md * Update src/scripts/disable-openid.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * changed error codes based on code rabbit review * fix for github auth * code review --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…B_BYLADEM1) to the 90-days sync limited list (actualbudget#528)
* 🔖 (25.1.0) * Remove used release notes * Update web package --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* chore: payee name for google pay * chore: revert notes to include all data * chore: release note * chore: fix tests
…budget#535) * Added special corner case * One additional validation + Release notes * Fixed duplicate code * Improved comment * Improved comment --------- Co-authored-by: spideraxal <[email protected]>
* ✨ add bank adapter for BOI * Remove console log
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (Array.isArray(accountId) != Array.isArray(startDate)) { | ||
| console.log(accountId, startDate); |
Check failure
Code scanning / CodeQL
Use of externally-controlled format string High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the problem, we need to ensure that the user-provided values are safely included in the format string. This can be achieved by using the %s specifier in the console.log statement and passing the accountId and startDate as additional arguments. This approach will prevent any format string vulnerabilities.
- Modify the
console.logstatement on line 79 to use the%sspecifier. - Pass the
accountIdandstartDateas additional arguments to theconsole.logfunction.
| @@ -78,3 +78,3 @@ | ||
| if (Array.isArray(accountId) != Array.isArray(startDate)) { | ||
| console.log(accountId, startDate); | ||
| console.log('accountId: %s, startDate: %s', accountId, startDate); | ||
| throw new Error( | ||
| @@ -84,3 +84,3 @@ | ||
| if (Array.isArray(accountId) && accountId.length !== startDate.length) { | ||
| console.log(accountId, startDate); | ||
| console.log('accountId: %s, startDate: %s', accountId, startDate); | ||
| throw new Error('accountId and startDate arrays must be the same length'); |
| ); | ||
| } | ||
| if (Array.isArray(accountId) && accountId.length !== startDate.length) { | ||
| console.log(accountId, startDate); |
Check failure
Code scanning / CodeQL
Use of externally-controlled format string High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the problem, we need to ensure that the user-provided input is not directly used in the format string. Instead, we should use a %s specifier in the format string and pass the untrusted data as corresponding arguments. This will prevent any unexpected format specifiers from causing issues.
Specifically, we will modify the console.log statements on lines 79 and 85 to use %s specifiers and pass the accountId and startDate variables as arguments.
| @@ -78,3 +78,3 @@ | ||
| if (Array.isArray(accountId) != Array.isArray(startDate)) { | ||
| console.log(accountId, startDate); | ||
| console.log('accountId: %s, startDate: %s', accountId, startDate); | ||
| throw new Error( | ||
| @@ -84,3 +84,3 @@ | ||
| if (Array.isArray(accountId) && accountId.length !== startDate.length) { | ||
| console.log(accountId, startDate); | ||
| console.log('accountId: %s, startDate: %s', accountId, startDate); | ||
| throw new Error('accountId and startDate arrays must be the same length'); |
| let username = null; | ||
| let password = null; | ||
| let baseUrl = null; | ||
| if (!accessKey || !accessKey.match(/^.*\/\/.*:.*@.*$/)) { |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
| const req = https.request(new URL(token), options, (res) => { | ||
| res.on('data', (d) => { | ||
| resolve(d.toString()); | ||
| }); | ||
| }); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the SSRF vulnerability, we need to ensure that the URL used in the https.request call is not directly influenced by user input. Instead, we should validate the token against a whitelist of allowed URLs or domains. This can be achieved by extracting the hostname from the URL and checking it against a predefined list of allowed hostnames.
- Extract the hostname from the
tokenURL. - Validate the hostname against a whitelist of allowed hostnames.
- If the hostname is not in the whitelist, reject the request.
| @@ -286,2 +286,9 @@ | ||
| const token = Buffer.from(base64Token, 'base64').toString(); | ||
| const url = new URL(token); | ||
| const allowedHostnames = ['allowed-hostname1.com', 'allowed-hostname2.com']; | ||
|
|
||
| if (!allowedHostnames.includes(url.hostname)) { | ||
| throw new Error('Invalid hostname in token URL'); | ||
| } | ||
|
|
||
| const options = { | ||
| @@ -292,3 +299,3 @@ | ||
| return new Promise((resolve, reject) => { | ||
| const req = https.request(new URL(token), options, (res) => { | ||
| const req = https.request(url, options, (res) => { | ||
| res.on('data', (d) => { |
| const req = https.request( | ||
| new URL(`${sfin.baseUrl}/accounts${queryString}`), | ||
| options, | ||
| (res) => { | ||
| let data = ''; | ||
| res.on('data', (d) => { | ||
| data += d; | ||
| }); | ||
| res.on('end', () => { | ||
| if (res.statusCode === 403) { | ||
| reject(new Error('Forbidden')); | ||
| } else { | ||
| try { | ||
| const results = JSON.parse(data); | ||
| results.sferrors = results.errors; | ||
| results.hasError = false; | ||
| results.errors = {}; | ||
| resolve(results); | ||
| } catch (e) { | ||
| console.log(`Error parsing JSON response: ${data}`); | ||
| reject(e); | ||
| } | ||
| } | ||
| }); | ||
| }, | ||
| ); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the problem, we need to ensure that the baseUrl used in the https.request call is validated against a whitelist of allowed URLs. This will prevent user-controlled input from being used directly in the URL, mitigating the risk of SSRF attacks.
- Create a whitelist of allowed base URLs.
- Validate the
baseUrlagainst this whitelist before using it in thehttps.requestcall. - If the
baseUrlis not in the whitelist, reject the request or handle it appropriately.
| @@ -262,2 +262,4 @@ | ||
|
|
||
| const allowedBaseUrls = ['https://api.simplefin.com', 'https://sandbox.simplefin.com']; | ||
|
|
||
| function parseAccessKey(accessKey) { | ||
| @@ -277,2 +279,8 @@ | ||
| baseUrl = `${scheme}//${rest}`; | ||
|
|
||
| if (!allowedBaseUrls.includes(baseUrl)) { | ||
| console.log(`Disallowed base URL: ${baseUrl}`); | ||
| throw new Error(`Disallowed base URL`); | ||
| } | ||
|
|
||
| return { |
| } | ||
|
|
||
| try { | ||
| await fs.writeFile(getPathForUserFile(fileId), req.body); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the problem, we need to ensure that the file path constructed from the user-provided fileId is safe and does not allow directory traversal or access to unintended files. We can achieve this by normalizing the path and ensuring it is contained within a predefined root directory.
- Define a root directory where user files are stored.
- Use
path.resolveto normalize the constructed file path. - Verify that the normalized path starts with the root directory path.
| @@ -10,2 +10,3 @@ | ||
| import { getPathForUserFile, getPathForGroupFile } from './util/paths.js'; | ||
| import path from 'path'; | ||
|
|
||
| @@ -232,3 +233,9 @@ | ||
| try { | ||
| await fs.writeFile(getPathForUserFile(fileId), req.body); | ||
| const ROOT_DIR = '/path/to/root/directory'; | ||
| let userFilePath = path.resolve(ROOT_DIR, getPathForUserFile(fileId)); | ||
| if (!userFilePath.startsWith(ROOT_DIR)) { | ||
| res.status(400).send('Invalid file path'); | ||
| return; | ||
| } | ||
| await fs.writeFile(userFilePath, req.body); | ||
| } catch (err) { |
| } | ||
|
|
||
| res.setHeader('Content-Disposition', `attachment;filename=${fileId}`); | ||
| res.sendFile(getPathForUserFile(fileId)); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. We can achieve this by normalizing the path using path.resolve and then checking that the normalized path starts with the root folder. This will prevent path traversal attacks by removing any ".." segments and ensuring the file path is within the intended directory.
- Import the
pathmodule. - Define a constant for the root directory where user files are stored.
- Normalize the file path using
path.resolve. - Check that the normalized path starts with the root directory.
- If the check fails, return a 403 status code.
| @@ -10,2 +10,3 @@ | ||
| import { getPathForUserFile, getPathForGroupFile } from './util/paths.js'; | ||
| import path from 'path'; | ||
|
|
||
| @@ -13,2 +14,4 @@ | ||
|
|
||
| const USER_FILES_ROOT = '/path/to/user/files'; | ||
|
|
||
| import { SyncProtoBuf } from '@actual-app/crdt'; | ||
| @@ -295,4 +298,9 @@ | ||
|
|
||
| const filePath = path.resolve(USER_FILES_ROOT, getPathForUserFile(fileId)); | ||
| if (!filePath.startsWith(USER_FILES_ROOT)) { | ||
| res.status(403).send('Access denied'); | ||
| return; | ||
| } | ||
| res.setHeader('Content-Disposition', `attachment;filename=${fileId}`); | ||
| res.sendFile(getPathForUserFile(fileId)); | ||
| res.sendFile(filePath); | ||
| }); |
| // Source: https://expressjs.com/en/guide/error-handling.html | ||
| return next(err); | ||
| } | ||
| console.log(`Error on endpoint ${req.url}`, err.message, err.stack); |
Check failure
Code scanning / CodeQL
Use of externally-controlled format string High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 15 days ago
To fix the problem, we should avoid directly embedding the untrusted req.url value in the format string. Instead, we can use a %s specifier in the format string and pass the req.url as a separate argument to ensure it is treated as a string. This approach will prevent any potential format string vulnerabilities.
| @@ -24,3 +24,3 @@ | ||
| } | ||
| console.log(`Error on endpoint ${req.url}`, err.message, err.stack); | ||
| console.log('Error on endpoint %s', req.url, err.message, err.stack); | ||
| res.status(500).send({ status: 'error', reason: 'internal-error' }); |
No description provided.