Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 4 additions & 0 deletions src/commands/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const serverCommand = new Command('server')
.option('--car-storage <path>', 'path for CAR file storage', './cars')
.option('--database <path>', 'path to SQLite database', './pins.db')
.option('--private-key <key>', 'private key for Synapse (or use PRIVATE_KEY env var)')
.option('--access-token <token>', 'bearer token required on all API requests (or use ACCESS_TOKEN env var)')
Comment thread
wjmelements marked this conversation as resolved.
Outdated

addNetworkOptions(serverCommand)
.addOption(
Expand All @@ -20,6 +21,9 @@ addNetworkOptions(serverCommand)
if (options.privateKey) {
process.env.PRIVATE_KEY = options.privateKey
}
if (options.accessToken) {
process.env.ACCESS_TOKEN = options.accessToken
}
// RPC URL takes precedence over network flag
if (options.rpcUrl) {
process.env.RPC_URL = options.rpcUrl
Expand Down
1 change: 1 addition & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export function createConfig(): Config {

// Synapse SDK configuration
privateKey: process.env.PRIVATE_KEY, // Required: Ethereum-compatible private key
accessToken: process.env.ACCESS_TOKEN,
rpcUrl, // Determined from RPC_URL, NETWORK, or default to calibration
Comment on lines 52 to 54
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

createConfig() now reads ACCESS_TOKEN, but the surrounding configuration docs only mention PRIVATE_KEY, RPC_URL, and NETWORK. Consider updating the comment/docs to include ACCESS_TOKEN (and what format is expected: the raw token value, not the Bearer prefix).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is obvious, but I've also done a lot of stuff with bearer tokens.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should move accessToken up to the "Application-specific" section cause this section is documented as the Synapse fields

// Storage paths
databasePath: process.env.DATABASE_PATH ?? join(dataDir, 'pins.db'),
Expand Down
1 change: 1 addition & 0 deletions src/core/synapse/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface Config {
port: number
host: string
privateKey: string | undefined
accessToken: string | undefined
rpcUrl: string
databasePath: string
carStoragePath: string
Expand Down
5 changes: 5 additions & 0 deletions src/filecoin-pinning-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ export async function createFilecoinPinningServer(
return
}

if (config.accessToken && token !== config.accessToken) {
await reply.code(401).send({ error: 'Invalid access token' })
Comment thread
wjmelements marked this conversation as resolved.
Outdated
return
Comment thread
wjmelements marked this conversation as resolved.
Outdated
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The new config.accessToken enforcement branch isn’t covered by tests right now. Please add coverage that verifies requests are rejected with 401 when a configured access token does not match, and accepted when it matches (and consider a test for missing/invalid header behavior when the access token is configured).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is some server test coverage on another branch (#376) that I would prefer to extend, instead of adding another test here. Fewer merge conflicts that way.


// Add user to request context
request.user = DEFAULT_USER_INFO
})
Expand Down
Loading