Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
Run project with
`npm run start`

Sign up to the NASA Open API to get your personal key.
Create a `.env` local file and store your new key there to use this app.
15 changes: 15 additions & 0 deletions app/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import express from "express";
import roverRouter from "../routes/roverRouter";

const app = express();

app.get("/", (req: any, res: any) => {
res.send(`Hello space traveller!
What are you exploring today?
A) /rovers = all rovers' info
B) /rovers/[your preferred rover's name]/photos = photos from that rover`)
});

app.use("/rovers", roverRouter);

export default app;
19 changes: 18 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"dependencies": {
"@types/express": "^5.0.0",
"axios": "^1.7.7",
"dotenv": "^16.4.5",
"express": "^4.21.1",
"nodemon": "^3.1.7",
"ts-node": "^10.9.2"
Expand All @@ -11,5 +12,12 @@
},
"devDependencies": {
"tsx": "^4.19.2"
}
},
"name": "marsappapi",
"version": "1.0.0",
"description": "Run project with `npm run start`",
"main": "index.js",
"keywords": [],
"author": "",
"license": "ISC"
}
101 changes: 101 additions & 0 deletions routes/roverRouter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import express from "express";
import axios from "axios";
const roverRouter = express.Router();

import dotenv from "dotenv";
dotenv.config({ path: './.env' });
const apiKey = process.env.NASA_API_KEY;
Comment on lines +6 to +7
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The code rabbit suggestion about checking if the API key exists is quite useful. Often times a missing environment variable is frustrating as you may think something is broken with the endpoint you've created, when in fact its just that you're missing a variable!

Adding in a small check to ensure its not empty is good for that and someone can fix it easily. If you start getting more and more environment variables, that's when you would potentially want a separate file that loads and validates env vars (then stores them in an object) :D


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

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve imports organization and add API validation

While the previous review covered Express types, there are additional improvements needed:

  1. Group imports by type (external, internal)
  2. Add type definitions for axios
  3. Add comprehensive API validation
+import { AxiosError, AxiosResponse } from 'axios';
 import axios from "axios";
 import express from "express";
+import { Router, Request, Response } from "express";
 import dotenv from "dotenv";

 dotenv.config({ path: './.env' });
 const apiKey = process.env.NASA_API_KEY;
+
+// Validate NASA API connectivity early
+async function validateApiKey() {
+  try {
+    await axios.get('https://api.nasa.gov/mars-photos/api/v1/rovers', {
+      params: { api_key: apiKey }
+    });
+  } catch (error) {
+    const axiosError = error as AxiosError;
+    if (axiosError.response?.status === 403) {
+      throw new Error('Invalid NASA_API_KEY');
+    }
+    throw new Error('Failed to validate NASA API connection');
+  }
+}
+
+validateApiKey().catch(error => {
+  console.error(error);
+  process.exit(1);
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import express from "express";
import axios from "axios";
const roverRouter = express.Router();
import dotenv from "dotenv";
dotenv.config({ path: './.env' });
const apiKey = process.env.NASA_API_KEY;
import { AxiosError, AxiosResponse } from 'axios';
import axios from "axios";
import express from "express";
import { Router, Request, Response } from "express";
import dotenv from "dotenv";
dotenv.config({ path: './.env' });
const apiKey = process.env.NASA_API_KEY;
// Validate NASA API connectivity early
async function validateApiKey() {
try {
await axios.get('https://api.nasa.gov/mars-photos/api/v1/rovers', {
params: { api_key: apiKey }
});
} catch (error) {
const axiosError = error as AxiosError;
if (axiosError.response?.status === 403) {
throw new Error('Invalid NASA_API_KEY');
}
throw new Error('Failed to validate NASA API connection');
}
}
validateApiKey().catch(error => {
console.error(error);
process.exit(1);
});

enum Cameras {
FHAZ = "FHAZ",
RHAZ = "RHAZ",
MAST = "MAST",
CHEMCAM = "CHEMCAM",
MAHLI = "MAHLI",
MARDI = "MARDI",
NAVCAM = "NAVCAM",
PANCAM = "PANCAM",
MINITES = "MINITES"
}

enum Rovers {
CURIOSITY = "Curiosity",
SPIRIT = "Spirit",
OPPORTUNITY = "Opportunity",
PERSEVERANCE = "Perseverance"
}

interface Camera {
id?: number;
name: string;
rover_id?: number;
full_name: string;
}

interface Rover {
id: number;
name: string;
landing_date: string;
launch_date: string;
status: string;
max_sol: number;
max_date: string;
total_photos: number;
cameras: Camera[];
}

interface Photo {
id: number;
sol: number;
camera: Camera;
img_src: string;
earth_date: string;
rover: Rover;
}

roverRouter.get('/', (req: any, res: any) => {
axios.get(`https://api.nasa.gov/mars-photos/api/v1/rovers`, {
params: {
api_key: apiKey
}
})
.then(response => {
res.send(response.data);
})
.catch(error => {
console.error(error);
res.render("error");
})
});

roverRouter.get('/:roverName/photos', (req: any, res: any) => {

const { roverName } = req.params;

const { camera, sol, page, paginationStart, paginationEnd } = req.query;

axios.get(`https://api.nasa.gov/mars-photos/api/v1/rovers/${roverName}/photos`, {
params: {
api_key: apiKey,
camera: camera,
page: page || 1,
sol: sol || 1000
}
})
.then(response => {
const photos: Photo[] = response.data.photos;
const urls: string[] = [];

photos.forEach(photo => {
urls.push(photo.img_src);
});

res.send(urls.slice(paginationStart - 1, paginationEnd));
})
.catch(error => {
console.error(error);
res.render("error");
})
});
Comment on lines +71 to +99
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Add security measures and update dependencies

The verification reveals several security concerns that need to be addressed:

  • No security headers or rate limiting is currently implemented
  • Missing essential security dependencies (helmet, express-rate-limit)
  • Express version 4.21.1 has known vulnerabilities
  • No input validation/sanitization for URL parameters

Required changes:

  • Update Express to version 4.21.2 or later to patch security vulnerabilities
  • Add helmet and express-rate-limit as dependencies
  • Implement input validation and sanitization as suggested in the review
  • Add security headers through helmet middleware
🔗 Analysis chain

Add rate limiting and security headers

While previous comments covered validation and pagination, additional security measures are needed:

  1. Implement rate limiting to prevent API abuse
  2. Add security headers
  3. Sanitize and validate URL parameters before use
+import rateLimit from 'express-rate-limit';
+import helmet from 'helmet';
+
+// Apply rate limiting
+const limiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 100 // limit each IP to 100 requests per windowMs
+});
+
+roverRouter.use(limiter);
+roverRouter.use(helmet());
+
 roverRouter.get('/:roverName/photos', (req: any, res: any) => {
+  // Sanitize URL parameters
+  const roverName = encodeURIComponent(req.params.roverName);
+  if (!Object.values(Rovers).includes(roverName as Rovers)) {
+    return res.status(400).json({ error: 'Invalid rover name' });
+  }
   
-  const { roverName } = req.params;
   // ... rest of the code

Let's verify the security implications:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for potential security issues in dependencies
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: NPM, package: "express") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
    }
  }
}'

# Check if rate limiting is implemented elsewhere
rg -l "rateLimit|express-rate-limit"

Length of output: 1203


Script:

#!/bin/bash
# Check if security headers or rate limiting is already implemented in the main app setup
rg -A 5 "app\.(use|set)\(" 

# Check for existing security-related dependencies in package files
fd "package.json" --exec cat {} 

# Check for any existing input validation or sanitization
ast-grep --pattern 'req.params.$_'

Length of output: 659


export default roverRouter;
13 changes: 3 additions & 10 deletions src/server.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import express from "express";
import app from "../app";

const app = express();
const port = 5000;

app.use(express.json());
const router = express.Router();

router.get('/', (req: any, res: any) => res.send('Hello world !'));
app.use('/', router);
const port = 3000;

app.listen(port, () => {
console.log(`Test backend is running on port ${port}`);
});
});